Redo finer-grained inline constant cache invalidation #5716
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
kddnewton
force-pushed
the
revert-5715-revert-5433-constant-invalidation
branch
2 times, most recently
from
fce6d41
to
f836cfa
Compare
kddnewton
force-pushed
the
revert-5715-revert-5433-constant-invalidation
branch
15 times, most recently
from
1f0e3ed
to
546c456
Compare
XrXr
reviewed
Mar 31, 2022
XrXr
reviewed
Mar 31, 2022
Some of the symbols had changed names and the script was no longer finding them.
This commit reintroduces finer-grained constant cache invalidation. After 8008fb7 got merged, it was causing issues on token-threaded builds (such as on Windows). The issue was that when you're iterating through instruction sequences and using the translator functions to get back the instruction structs, you're either using `rb_vm_insn_null_translator` or `rb_vm_insn_addr2insn2` depending if it's a direct-threading build. `rb_vm_insn_addr2insn2` does some normalization to always return to you the non-trace version of whatever instruction you're looking at. `rb_vm_insn_null_translator` does not do that normalization. This means that when you're looping through the instructions if you're trying to do an opcode comparison, it can change depending on the type of threading that you're using. This can be very confusing. So, this commit creates a new translator function `rb_vm_insn_normalizing_translator` to always return the non-trace version so that opcode comparisons don't have to worry about different configurations. [Feature #18589]
kddnewton
force-pushed
the
revert-5715-revert-5433-constant-invalidation
branch
from
546c456
to
5b223bf
Compare
Comment on lines
+103
to
+108
| begin | ||
| Container.private_constant :CONST1, :CONST2 | ||
| rescue NameError | ||
| end | ||
|
|
||
| const1 |
There was a problem hiding this comment.
If :CONST1 and :CONST2 have the positions inverted, const1 in line 108 will return 1.
There was a problem hiding this comment.
Right, CONST2 doesn't exist, so when it's reversed private_constant raises NameError before changing the visibility of CONST1. On Ruby 3.1 private_constant behaves like this too.
There was a problem hiding this comment.
Gotcha. Thank you for the explanation.
|
@kddnewton @XrXr /cc @tenderlove Can you update |
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.
Brings back the constant invalidation code from https://bugs.ruby-lang.org/issues/18589, now that the underlying BIN comparison bug is fixed by #5741.