-
Notifications
You must be signed in to change notification settings - Fork 0
Optimize array! DSL
#14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize array! DSL
#14
Conversation
| _scope{ array! value, &block } | ||
| _scope{ _array value, &block } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
json.set! :posts, posts do |post|
json.extract! post, :id, :body
endruby 3.4.4 (2025-05-14 revision a38531fd3f) +YJIT +PRISM [arm64-darwin24]
Warming up --------------------------------------
before 74.247k i/100ms
after 77.990k i/100ms
Calculating -------------------------------------
before 755.086k (± 1.2%) i/s (1.32 μs/i) - 3.787M in 5.015504s
after 802.630k (± 1.4%) i/s (1.25 μs/i) - 4.055M in 5.053783s
Comparison:
after: 802630.0 i/s
before: 755086.0 i/s - 1.06x slower
Calculating -------------------------------------
before 800.000 memsize ( 520.000 retained)
11.000 objects ( 4.000 retained)
0.000 strings ( 0.000 retained)
after 760.000 memsize ( 520.000 retained)
10.000 objects ( 4.000 retained)
0.000 strings ( 0.000 retained)
Comparison:
after: 760 allocated
before: 800 allocated - 1.05x more
| _scope{ array! value, *args } | ||
| _scope{ _array value, args } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
json.set! :posts, posts, :id, :bodyruby 3.4.4 (2025-05-14 revision a38531fd3f) +YJIT +PRISM [arm64-darwin24]
Warming up --------------------------------------
before 69.118k i/100ms
after 80.906k i/100ms
Calculating -------------------------------------
before 718.268k (± 1.5%) i/s (1.39 μs/i) - 3.594M in 5.005122s
after 837.174k (± 1.5%) i/s (1.19 μs/i) - 4.207M in 5.026501s
Comparison:
after: 837173.8 i/s
before: 718267.7 i/s - 1.17x slower
Calculating -------------------------------------
before 832.000 memsize ( 520.000 retained)
8.000 objects ( 4.000 retained)
0.000 strings ( 0.000 retained)
after 640.000 memsize ( 520.000 retained)
7.000 objects ( 4.000 retained)
0.000 strings ( 0.000 retained)
Comparison:
after: 640 allocated
before: 832 allocated - 1.30x more
mscrivo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can also include benchmarks against upstream_main to show how all of our optimizations compare to stock jbuilder thus far.
Not necessary for this PR, but this would be really interesting to see!
Insomniak47
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! Also I'd love to see us start to build out a corpus of tests that's a bit more edge-casey so that we can see that we're not evaluating anything that's too trivial and losing out on more complex cases. All these look good AFAICT
This optimizes the
array!DSL to run faster with less memory usage. The PR may look a bit bigger than it actually is, but a summary of the changes:one?. This shows up as a hotspot for us, and it isn't actually needed. The intent here was to check if a single argument was provided toarray!to determine whether or not a partial should be rendered. It's actually just faster to check if the first argument is aHashthan it is to call out toone?first, because the latter is an O(n) operation.::Kernel.block_given?. BecauseJbuilderis aBasicObject, it does not have direct access toblock_given?, so it must explicitly call out to theKernelmodule instead. This is actually slower than simply explicitly checking if theblockargument exists.array!, which splats*argsinto an allocatedArrayeach time. The PR introduces_arrayto be used internally, which saves on this allocation. This is similar to what was done in Optimize internalextract!calls to save on memory allocation #7 forextract!.EMPTY_ARRAYto save on allocating a new empty array (ie.[]) each time an empty collections are rendered.The leanest way to benchmark the
array!DSL is withThis gets the main changes running the most frequently under the benchmark without it being diluted by other things. Results look good so far!
There are a few other spots that are now also taking advantage of these optimizations. Those have been annotated below.
Please note that all benchmarks are being compared to our fork's
mainbranch, so they capture the differences introduced by this PR. If you like, I can also include benchmarks againstupstream_mainto show how all of our optimizations compare to stockjbuilderthus far.(Still poking around the benchmarks. I will include them in the PR soon.)