Improve cache hits and JIT friendliness #486
Conversation
|
I'm happy to fix the style issues, but I believe they were in the pre-existing code. Please let me know, and I'll fix them. 😃 |
|
That's fine, it was obvious to me that you aimed for the minimal amount of changes. That made it easy to read the diff. (It is a bit unfortunate that there's a bot with rules that aren't consistent with the existing code; should be addressed in a separate PR IMHO (not saying it is anything you need to do)) I'm thinking about the change from guards/early returns, many lines changed just because they needed indenting. Could that be avoided or is this how it need to be in order to be performant? Just trying to keep future |
|
@dentarg nope, we don't need to do any indentation changes. I just did them because I guess that's the way it's supposed to be (but I was basically doing a mechanical replacement, and I have no opinion). I can change them to Btw, if you add |
|
In retrospect, I probably should have done the |
This commit makes Addressable::URI more cache friendly. It changes the internals to always define all instance variables, and uses a sentinel object in the cases where we need to cache a computation that could possibly result in nil.
`defined?` always returns true for local variables, so there's no need to check these
|
Ok, the diff should be much smaller now. Thank you! |
|
I know about Thanks for these changes, they look good to me! |
|
Thank you! |
Ref: sporkmonger#486 Following the introduction of `NONE` flags, `Addressable::URI` instance can no longer be correctly serialized with YAML. It appear to work, but `NONE` is serialized as `!ruby/object {}`, so when the YAML is parsed back, the attributes are assigned a distinct anonymous object.
Ref: sporkmonger#486 Following the introduction of `NONE` flags, `Addressable::URI` instance can no longer be correctly serialized with YAML. It appear to work, but `NONE` is serialized as `!ruby/object {}`, so when the YAML is parsed back, the attributes are assigned a distinct anonymous object.
Ref: #486 Following the introduction of `NONE` flags, `Addressable::URI` instance can no longer be correctly serialized with YAML. It appear to work, but `NONE` is serialized as `!ruby/object {}`, so when the YAML is parsed back, the attributes are assigned a distinct anonymous object. Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
Hi,
This PR adds a few commits to improve inline cache hits for instance variables as well as improve JIT friendliness. Removing and adding instance variables is very bad for JIT compilers, so this PR removes the "remove" calls. When we want to remove any caches or precomputed values, we simply set the instance variable to
nil, or to the sentinel object. We don't have to check whether or not the IV is defined before setting the value, we can just set it.This PR also removes some calls to
defined?that didn't do anything.defined?will always return true for local variables, so there was no need to check it.Finally, since we can stop checking
defined?on IVs, I was able to convert some of the reader methods to just useattr_reader.Thanks!