[pull] master from ruby:master#1084
Merged
Merged
Conversation
…simplify the transition from a JSON_PHASE_VALUE to the next phase. ruby/json@887274e642
Extracted from: ruby/json#994 Modern compilers shouldn't have problem computing `strlen` at compile time and generating the same code. ruby/json@b07f74bd73
`LoadField` has an associated `return_type` field that's set depending on the type of load. When an object's shape transitions its storage strategy (embedded->heap), the return type may change (e.g., `BasicObject` to `CPtr`). If that shape layout transition is sandwiched between two `LoadField` inside the same block, it's possible we will see two different return types when reading from the same offset of the same object. Load-store forwarding will rightfully eliminate the second `LoadField`. However, had it not been removed, we'd have two resulting bit-for-bit equal values with two different associated types. That confuses the type checker and so semantically valid code will panic with a `MismatchedOperandType`. I ran across this while working on GH-16966. I've pulled it out here to make review easier and because the fix doesn't touch any inlining code. We haven't run into this thus far because a same-block shape storage transition emits a `SetIvar`, which has a broad enough effect that load-store forwarding reset its cache. If anything along that chain changes then we could get into this state without inlining. It's a lot to keep track of so I think we should tackle this broadly. I chatted with @tekknolagi about the general problem. A simple or obvious solution did not present itself. This PR does not fix the broader problem; it temporarily patches over the `MismatchedOperandType` by preserving duplicate loads if the return value would be different. This is not ideal and will need to be reverted when a comprehensive solution is implemented. But, it's enough to satisfy the type checker.
Object tracing listens to the NEWOBJ hook to see all objects allocated while it is active. Previously it also enabled a FREEOBJ tracepoint to drop each object's record as the object was freed. However the FREEOBJ tracepoint only fires while tracing is active. An object recorded during tracing can be freed after tracing stops, and that free was missed, leaving a stale record in object_table keyed by a freed object. These dangling keys are unsafe during compaction, which was previously mitigated with rb_gc_pointer_to_heap_p (which I'd like to stop using). These dangling keys could also become incorrectly associated with a new object allocated in its place. Instead we can declare object_table's keys as weak references. The GC then invokes our weak reference callback on every collection, whether or not tracing is running, letting us reap the records of objects about to be freed. This callback runs before the FREEOBJ hook, which makes the FREEOBJ tracepoint unnecessary.
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )