Increase activerecord benchmark size

[casperisfine]

The Active Record benchmark doesn't do a whole lot. It load a single record without any association and just read a few fields. It also always fetch directly from the database.

Here we add a single extra association, enable the query cache to remove IOs entirely, and access a few more attributes to exercise the casting code more

Before:

interp: ruby 3.3.1 (2024-04-23 revision c56cd86388) [arm64-darwin23]
yjit: ruby 3.3.1 (2024-04-23 revision c56cd86388) +YJIT [arm64-darwin23]

------------  -----------  ----------  ---------  ----------  ------------  -----------
bench         interp (ms)  stddev (%)  yjit (ms)  stddev (%)  yjit 1st itr  interp/yjit
activerecord  29.4         2.8         13.2       2.7         1.55          2.23
------------  -----------  ----------  ---------  ----------  ------------  -----------

After:

interp: ruby 3.3.1 (2024-04-23 revision c56cd86388) [arm64-darwin23]
yjit: ruby 3.3.1 (2024-04-23 revision c56cd86388) +YJIT [arm64-darwin23]

------------  -----------  ----------  ---------  ----------  ------------  -----------
bench         interp (ms)  stddev (%)  yjit (ms)  stddev (%)  yjit 1st itr  interp/yjit
activerecord  165.8        2.6         71.2       2.6         1.93          2.33
------------  -----------  ----------  ---------  ----------  ------------  -----------

Profile before:

Profile after:

You can see that before the top leafs were mundane things such as allocating record, Sqlite3 and accessing various thread local state. After the change the top elements are more relevant things such as attribute access and casting, association mapping, etc.

NB: Given that activerecord is marked as headline, and this is quite a radical change, perhaps it can just be a new benchmark, called activerecord2 or activerecord-read-heavy or something?

[casperisfine]

Alternatively, since Rails 7.2 should go out soon, we could wait and do the upgrade at the same time.

[etiennebarrie]

I think it's fine to change the benchmarks over time, like in 03f091c, the benchmark got twice as fast due to changes to the benchmark, not YJIT or ruby, and it shows on the all-time view:

https://speed.yjit.org/timeline-deep.html#activerecord

[maximecb]

Thanks Jean this looks a lot nicer. Let's merge it. We want the headline benchmarks to be reasonably fair/representative 👍

[eregon]

The change looks good to me.
Two thoughts:

• the new code doesn't generate a string anymore, but in most Rails apps one would need to generate a string from these attributes (even if it's JSON and not HTML). Notably Time formatting is not cheap, but something many Rails apps will do. I think that would be worth tweaking to be more representative and also avoid e.g. a JIT noticing title is called 3 times and just reading it once or zero times because it's unused.
• Using the query cache here doesn't seem representative, a real app wouldn't do the same query (10 * iterations) times in a single request. I thought it was on purpose this benchmark measures SQlite since a real app would need to query the database (sqlite OTOH is not typically the DB used in production but is certainly more practical than mysql2/pg).

[maximecb]

Those are fair points.

I think it would make sense to generate a string as Benoit suggests.

Wrt query cache, ideally we'd generate a lot of posts (time budget < 10s to generate), and then request random posts. Some of them would get cached, some not.

[casperisfine]

in most Rails apps one would need to generate a string from these attributes

This is generally true. But that's more the job of "rails-bench" or similar to simulate a full Rails request. If you simulate templating, you might as well also do the controller etc etc.

Here the focus is Active Record itself. I think it make sense to only exercise Active Record APIs.

Using the query cache here doesn't seem representative

Similarly, IMO the goal is to show what's left to optimize. We can totally do the SQLite query I don't mind, but that's just gonna add some overhead that JITs can't do anything about. I guess it somewhat exercise C bindings, so why not.

Some of them would get cached, some not.

I'd be concerned it would increase variance between runs.

[eregon]

This is generally true. But that's more the job of "rails-bench" or similar to simulate a full Rails request. If you simulate templating, you might as well also do the controller etc etc.

Here the focus is Active Record itself. I think it make sense to only exercise Active Record APIs.

Right, agreed a controller etc is too much.
OTOH I think it's not so rare that Active Record is used outside of Rails.
And regardless where it's used, it's never going to query the database and do nothing with the data, at minimum there would be some processing on the data.
Transforming to a string seems a very common processing step (also happens e.g. just when printing data).

I see it as like microbenchmark vs something representative of a real usage of ActiveRecord. Given this is not in microbenchmarks, I think a simple processing step is good.

Similarly, IMO the goal is to show what's left to optimize. We can totally do the SQLite query I don't mind, but that's just gonna add some overhead that JITs can't do anything about. I guess it somewhat exercise C bindings, so why not.

It's part of real usages of ActiveRecord so I wouldn't remove it. And if the JIT can optimize anything around calling C methods it might be very much worthwhile (also there might be things to optimize in the sqlite3 C ext).
(FWIW TruffleRuby < 24.0 runs C exts on Sulong, so it JIT compiles C code)

Some of them would get cached, some not.

I'd be concerned it would increase variance between runs.

Also since the cache is global it would cause variance between iterations (not sure if that's what you meant by runs).
I think in a well-behaved app the query cache gets only cache misses, or very few hits overall (compared to numbers of queries), and so it seems more representative to disable it, but correct me if I'm wrong.

[maximecb]

I think in a well-behaved app the query cache gets only cache misses, or very few hits overall (compared to numbers of queries), and so it seems more representative to disable it, but correct me if I'm wrong.

Is the cache part of sqlite itself? If so, the cache hit rate should actually be fairly high in a real server. I agree that there are concerns with variance. 🤔

What's the performance like with the cache on vs off?

With respect to building a string to output, I do think it might make some sense to do that. If there is no output, a sufficiently advanced JIT may be able to eliminate the calls.

[eregon]

Is the cache part of sqlite itself?

It's this: https://guides.rubyonrails.org/caching_with_rails.html#sql-caching and per request, so normally it wouldn't hit but because we don't really have a request here it becomes effectively a global cache.