Make Object pointer-sized#23542
Open
SuchAFuriousDeath wants to merge 12 commits intoruffle-rs:masterfrom
Open
Conversation
Collaborator
|
This worsens a Master: With this PR: |
Moves `new` and `custom_new` from the Erased-defaulted `impl<'gc>` block onto the generic `impl<'gc, K>` block so that each caller's inferred `K` (taken from the field or let binding context) determines the resulting `ScriptObjectData<'gc, K>` type. No callers need to change today: every existing call site assigns into a `ScriptObjectData<'gc>` field which picks up the `K = Erased` default exactly as before. Also switches the repr from `align(8)` to `#[repr(C, align(8))]` so that every `ScriptObjectData<'gc, K>` has a guaranteed identical in-memory layout regardless of `K`, and adds an `erase_kind` inherent helper that casts `Gc<Self>` to `Gc<ScriptObjectData<'gc, Erased>>` on the back of that guarantee. Per-variant migrations will use the helper in their `gc_base` implementations once their `base` fields become typed.
For every variant, `base: ScriptObjectData<'gc>` becomes `base: ScriptObjectData<'gc, kind::X>`, and `gc_base` wraps the `HasPrefixField::as_prefix_gc` call in `erase_kind`. `ScriptObject`'s direct `Gc<ScriptObjectData<'gc>>` inner becomes `Gc<ScriptObjectData<'gc, kind::ScriptObject>>`, with the one internal wrap-into-ScriptObjectWrapper site erasing explicitly. Constructor call sites are unchanged; the type parameter is inferred from the destination field.
Adds a runtime `kind: ObjectKind` field, tightens `new` and `custom_new` to require `K: Kind`, and sets `kind: K::ID` in the struct literal. Since every variant's `base` field is typed with its matching kind marker, the stored tag is structurally guaranteed to match the variant; no caller passes it and no caller can get it wrong.
Every type is trivially a prefix of itself. The cast is a no-op — no alignment check or offset assertion needed. Needed by the upcoming `ObjectVariant` impl for `ScriptObject`, whose `Data` type is `ScriptObjectData<'gc, kind::ScriptObject>` itself (with no wrapper `*Data` struct), so the `HasPrefixField<Data>` bound on `ObjectVariant::Data` can only be satisfied by an identity impl.
Adds a `tagged_trait_object` proc macro (sibling of `enum_trait_object`) that emits a `#[repr(transparent)]` struct wrapping `Gc<ScriptObjectData<Erased>>` instead of an enum. Method dispatch matches on `self.0.kind()` and casts through `crate::avm2::object::cast::downcast_unchecked`; `From<Variant>` impls go through `cast::upcast`. `Object` itself is switched to this representation, making it pointer-sized (8 bytes) rather than 16 bytes. `impl_downcast_methods!` and `define_weak_enum!` are updated to cast through the new helpers instead of matching enum variants.
Replaces the `WeakObject` enum with a `#[repr(transparent)]` struct wrapping `GcWeak<ScriptObjectData<Erased>>`. Upgrade and downgrade become trivial pointer-manipulation operations — no variant dispatch is needed, because the kind tag stays with the live allocation and an upgraded `WeakObject` yields an `Object` directly. Removes the `define_weak_enum!` macro and the 45 typed `\w+ObjectWeak` wrappers it relied on; none were used outside the enum definition.
…in the first cache line `#[repr(C)]` lays fields out in declaration order, so the previous ordering put the large `RefLock<DynamicMap<…>>` `values` field first and pushed `slots: Box<[Lock<Value<'gc>>]>` out to offset 64 — the second cache line of `GcBoxInner`'s value region. `op_get_slot` / `op_set_slot` then needed an extra cache line per access, costing measurable performance. Move `slots` and `vtable` to the front (right after `kind`) so that they share the first cache line with the GC header reads. Layout is otherwise unaffected — `kind` stays at offset 0 (required by the `tagged_trait_object` cast machinery), and `_kind: PhantomData` stays at the end.
6eebeb9 to
b563499
Compare
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 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 is a working preview of the long-awaited (and promised) Object refactor.
The design is 90% me + 10% advice from Codex to fill in the gaps that I couldn't quite get right. Most of the actual code changes were done by Claude Code, because they're mostly mechanical, I iterated with him until I was reasonably happy with everything.
I'm not fully satisfied with the design and I don't expect this to get merged without any adjustments but I have to post it here to get more eyes on it, or it won't get finished at all.
Imperfections:
As a sidenote, with the traits that currently exist in this PR, I think that handles (Stuff like ObjectHandle, NetStreamHandle, ScriptObjectHandle, MovieClipHandle and FileReferenceObjectHandle) could be generalized rather than having one type for each stashable Object type. So that's potentially something nice for the future.
Also, review commit-by-commit to preserve sanity.