Avoid defining singleton methods #62
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This supports overriding `#methods`.
This is done only for strict backward compatibility. It may not be needed for practical backward compatibility.
Don't define singleton methods unless they are needed to override existing methods. Otherwise, just rely on `method_missing` and `respond_to_missing?`.
All of the benchmarks except for 100x reads show a performance
improvement. If you are reading the data repeatedly, it's still much
better to use Struct or Data.
```
$ benchmark-driver benchmarks/ostruct.yml
Warming up --------------------------------------
new 7.047k i/s - 7.711k times in 1.094212s (141.90μs/i)
attr_write_read 4.904k i/s - 4.910k times in 1.001289s (203.93μs/i)
index_write_attr_read 6.346k i/s - 6.963k times in 1.097167s (157.57μs/i)
null_reads 79.488k i/s - 85.250k times in 1.072495s (12.58μs/i)
10x_reads 6.651k i/s - 7.282k times in 1.094836s (150.35μs/i)
100x_reads 5.136k i/s - 5.150k times in 1.002780s (194.71μs/i)
Calculating -------------------------------------
v0.6.0 local
new 6.938k 33.067k i/s - 21.141k times in 3.047342s 0.639334s
attr_write_read 4.886k 14.021k i/s - 14.711k times in 3.010686s 1.049232s
index_write_attr_read 6.270k 20.225k i/s - 19.039k times in 3.036489s 0.941373s
null_reads 78.351k 77.187k i/s - 238.462k times in 3.043491s 3.089414s
10x_reads 6.518k 13.306k i/s - 19.953k times in 3.061390s 1.499576s
100x_reads 4.454k 2.078k i/s - 15.407k times in 3.459125s 7.414447s
Comparison:
new
local: 33067.2 i/s
v0.6.0: 6937.5 i/s - 4.77x slower
attr_write_read
local: 14020.7 i/s
v0.6.0: 4886.3 i/s - 2.87x slower
index_write_attr_read
local: 20224.7 i/s
v0.6.0: 6270.1 i/s - 3.23x slower
null_reads
v0.6.0: 78351.5 i/s
local: 77186.8 i/s - 1.02x slower
10x_reads
local: 13305.8 i/s
v0.6.0: 6517.6 i/s - 2.04x slower
100x_reads
v0.6.0: 4454.0 i/s
local: 2078.0 i/s - 2.14x slower
```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR only creates new singleton methods when they are needed for overwriting an existing instance method. It should be mostly backward-compatible with the current OpenStruct implementation, but I suspect there are some minor incompatibilities (which may be significant).
I believe this should generally be a significant performance improvement. I'm guessing that OpenStruct objects are generally ephemeral and their attributes are generally read fewer than a dozen times on average. In my benchmarks, that wasn't enough for the benefits of defining singleton methods to outweigh the costs. All of the benchmarks I created, except for 100x reads, showed the same or better performance.
And that's mostly ignoring any non-local costs to constantly busting method caches. This PR should almost completely remove that cost.