-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wasmtime(gc): Add support for supertypes and finality #8595
Wasmtime(gc): Add support for supertypes and finality #8595
Conversation
a261664
to
1feacc6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
@@ -435,4 +437,19 @@ impl TypeConvert for WasmparserTypeConverter<'_> { | |||
UnpackedIndex::RecGroup(_) => unreachable!(), | |||
} | |||
} | |||
|
|||
fn lookup_type_index(&self, index: wasmparser::UnpackedIndex) -> EngineOrModuleTypeIndex { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the above method delegate to this new method to start off? (looks like they both start with all the same code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't quite work out because while both arms of the above method start with producing an EngineOrModuleIndex
, and that happens in the same way as this method, the arms continue by reusing intermediate steps from how the EngineOrModuleIndex
was produced in the code that follows after the index has been constructed. This makes it subtly difficult to pull apart or reuse.
// Therefore, if we have the path to the root for each type (we do) then | ||
// we can simply check if `sup` is at index `supertypes(sup).len()` | ||
// within `supertypes(sub)`. | ||
let inner = self.0.read(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC you had a scheme where a lock wasn't required from within wasm, right? (outside of wasm seems totally fine, just wanted to confirm for in-wasm)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the common case of checking subtyping of instances of types defined in this module, yes.
I don't have a plan for how to avoid locks in the terrible case of new external subtypes created dynamically:
- create module
A
with non-final typeT
- instantiate module
A
- create module
B
with typeU <: T
- instantiate module B
- instantiate module
B
- instantiate object of type
U
- pass object to instance
A
for subtyping check
(Note that module B
could also be the host using the public API)
My thinking is that we can just fall back to locking the type registry in the cross-module-subtyping case. I think there are some more tricks we can play with InstancePre
s to make cross-module-subtyping fast for all types that exist at InstancePre
creation time, but for new subtypes created during runtime we will always need to fall back to locking (or equivalent) AFAICT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok makes sense, I mostly wanted to confirm there was a means of intra-module checks that didn't require locking.
Subscribe to Label Action
This issue or pull request has been labeled: "wasmtime:api", "winch"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
This means that the storage for the vecs in each tree is, in total, in I think there's a cheaper
EDIT: And I guess if the types are already stored in some other structure(s), there's actually no need for a separate vector for this information; the primary representation of the type can just have those extra fields for: tree ID, index in tree, max descendent index. There's a good chance I've misunderstood something (e.g. how the trees are updated?), but if there's nothing else special about these trees then maybe this optimisation is applicable! 🙏 |
@jeffparsons, thanks for digging in here! Always appreciate a second pair of eyes.
It is indeed a time-space tradeoff, but as-implemented it can actually be even worse than However, implementations are allowed to define their own limits on the maximum depth a subtyping chain may have. And for JS embeddings, it is additionally tightened from "some implementation limit" to https://webassembly.github.io/gc/js-api/index.html#limits Anyways, the point is that this limit provides bounds on the total memory overhead associated with these supertype vectors, preventing worst-case scenarios. Finally, my understanding is that it is pretty much expected that engines implement subtyping checks with this technique, FWIW. For example, here is SpiderMonkey's docs about doing this same thing: https://searchfox.org/mozilla-central/rev/ee2ad260c25310a9fbf96031de05bbc0e94394cc/js/src/wasm/WasmTypeDef.h#480-541 This is a good reminder to double check that we are actually enforcing the subtyping depth limits in validation, though :-p
If I understand correctly, this is storing the entry and exit indices of a depth-first traversal of the tree and then subtype checking checks that (a) we are looking at the same tree and (b) that the subtype's index (doesn't matter if we check entry vs exit index) is within the supertype's range, correct? This would indeed work very nicely in the context of a single Wasm module and a static set of types. But, as you alluded to, things get problematic when we consider updates. GC objects can be passed between modules, and a non-final type from module A can be subtyped by another type in module B and then an instance of that subtype can be passed from B to A, and A might want to do subtyping checks on this instance even though it never defined the exact type of that instance. This necessitates canonicalizing types across modules. So, even though from a single Wasm module's perspective the type set is static, from an engine perspective we are constantly dynamically adding and removing types from the same deduplicated, canonicalized types registry as modules are loaded or unloaded (or when the host defines/drops types via the public Wasmtime API). And therefore our type hierarchies and their trees aren't static, and must support having subtrees grafted on and pruned away at any moment. That means we would need to recompute potentially all those depth-first traversal indices on every mutation, an So I don't think this approach is something we can use in this case, unfortunately. |
1feacc6
to
c617f51
Compare
This is the final type system change for Wasm GC: the ability to explicitly declare supertypes and finality. A final type may not be a supertype of another type. A concrete heap type matches another concrete heap type if its concrete type is a subtype (potentially transitively) of the other heap type's concrete type. Next, I'll begin support for allocating GC structs and arrays at runtime. I've also implemented `O(1)` subtype checking in the types registry: In a type system with single inheritance, the subtyping relationships between all types form a set of trees. The root of each tree is a type that has no supertype; each node's immediate children are the types that directly subtype that node. For example, consider these types: class Base {} class A subtypes Base {} class B subtypes Base {} class C subtypes A {} class D subtypes A {} class E subtypes C {} These types produce the following tree: Base / \ A B / \ C D / E Note the following properties: 1. If `sub` is a subtype of `sup` (either directly or transitively) then `sup` *must* be on the path from `sub` up to the root of `sub`'s tree. 2. Additionally, `sup` *must* be the `i`th node down from the root in that path, where `i` is the length of the path from `sup` to its tree's root. Therefore, if we maintain a vector containing the path to the root for each type, then we can simply check if `sup` is at index `supertypes(sup).len()` within `supertypes(sub)`.
c617f51
to
5db5a7a
Compare
This is the final type system change for Wasm GC: the ability to explicitly declare supertypes and finality. A final type may not be a supertype of another type. A concrete heap type matches another concrete heap type if its concrete type is a subtype (potentially transitively) of the other heap type's concrete type.
Next, I'll begin support for allocating GC structs and arrays at runtime.
I've also implemented
O(1)
subtype checking in the types registry:In a type system with single inheritance, the subtyping relationships between all types form a set of trees. The root of each tree is a type that has no supertype; each node's immediate children are the types that directly subtype that node.
For example, consider these types:
These types produce the following tree:
Note the following properties:
If
sub
is a subtype ofsup
(either directly or transitively) thensup
must be on the path fromsub
up to the root ofsub
's tree.Additionally,
sup
must be thei
th node down from the root in that path, wherei
is the length of the path fromsup
to its tree's root.Therefore, if we maintain a vector containing the path to the root for each type, then we can simply check if
sup
is at indexsupertypes(sup).len()
withinsupertypes(sub)
.