-
Notifications
You must be signed in to change notification settings - Fork 332
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
Speedup #generate_json by using case/when/end #674
Conversation
That's the one case where we had a failing test for it, but does that mean we should special case it? I'm open to say only String subclasses have that behavior, but then all 3 implementations should match behavior. Could you add some tests to make sure we converge them? Also I'm very surprised |
I'll rebase and see if the added tests fail on this PR or if they are fine. |
* if/elsif comparing `obj.class` is significantly slower: ruby#668 (comment) * The only case where an exact class check is needed so far is for String (ruby#667). * Before: $ ruby -Ilib:ext benchmark/standalone.rb dump pure JSON::Pure::Generator truffleruby 24.2.0-dev-07b978e4, like ruby 3.2.4, Oracle GraalVM JVM [x86_64-linux] Calculating ------------------------------------- JSON.dump(obj) 6.426k (± 5.9%) i/s (155.62 μs/i) - 32.395k in 5.064479s JSON.dump(obj) 6.380k (± 7.4%) i/s (156.73 μs/i) - 31.806k in 5.021304s JSON.dump(obj) 6.276k (±10.5%) i/s (159.33 μs/i) - 31.217k in 5.060762s JSON.dump(obj) 6.450k (± 7.0%) i/s (155.05 μs/i) - 32.395k in 5.059538s JSON.dump(obj) 6.413k (± 6.2%) i/s (155.93 μs/i) - 32.395k in 5.081573s * After: $ ruby -Ilib:ext benchmark/standalone.rb dump pure JSON::Pure::Generator truffleruby 24.2.0-dev-07b978e4, like ruby 3.2.4, Oracle GraalVM JVM [x86_64-linux] Calculating ------------------------------------- JSON.dump(obj) 8.237k (± 5.0%) i/s (121.41 μs/i) - 41.600k in 5.069507s JSON.dump(obj) 8.179k (± 5.1%) i/s (122.26 μs/i) - 40.768k in 5.002035s JSON.dump(obj) 8.147k (± 7.9%) i/s (122.74 μs/i) - 40.768k in 5.044840s JSON.dump(obj) 8.137k (± 6.9%) i/s (122.90 μs/i) - 40.768k in 5.048690s JSON.dump(obj) 8.112k (±10.2%) i/s (123.27 μs/i) - 39.936k in 5.023502s
ac44975
to
82d21f0
Compare
Tests seem to pass after rebase from a local test run. |
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.
If tests pass, then that's good for me.
…es subclasses Ref: ruby/json#674 Ref: ruby/json#668 The behavior on such case it quite unclear, the goal here is to figure out whatever was the behavior on Cext version of `json 2.7.0` and get all implementations to converge. We can then decide to make them all behave differently if we so wish. ruby/json@614921dcef
…es subclasses Ref: ruby/json#674 Ref: ruby/json#668 The behavior on such case it quite unclear, the goal here is to figure out whatever was the behavior on Cext version of `json 2.7.0` and get all implementations to converge. We can then decide to make them all behave differently if we so wish. ruby/json@614921dcef
…es subclasses Ref: ruby/json#674 Ref: ruby/json#668 The behavior on such case it quite unclear, the goal here is to figure out whatever was the behavior on Cext version of `json 2.7.0` and get all implementations to converge. We can then decide to make them all behave differently if we so wish. ruby/json@614921dcef
obj.class
is significantly slower: JSON.generate: call to_json on String subclasses #668 (comment)