Stream: git-wasmtime

Topic: wasmtime / PR #10463 Do not reorder/pack struct fields


view this post on Zulip Wasmtime GitHub notifications bot (Mar 24 2025 at 23:07):

fitzgen requested dicej for a review on PR #10463.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 24 2025 at 23:07):

fitzgen requested wasmtime-core-reviewers for a review on PR #10463.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 24 2025 at 23:07):

fitzgen opened PR #10463 from fitzgen:issue-10459 to bytecodealliance:main:

We were not considering that when a type T is a subtype of type U, that all of
the fields that T and U share must be at the same offsets. Our
reordering/packing algorithm did not consider this. So we stop reordering struct
fields for now.

This also means that we cannot guarantee that all GC refs in an object are
contiguously packed anymore. Instead, we maintain a map keyed by
VMSharedTypeIndex, whose value is all the offsets of outgoing GC
references for instances of that type. Tracing looks up the current object's
type in this map, and then extracts each of the GC edges.

This new tracing scheme means that we no longer need to store the number of
outgoing edges in the GC object header, and can put the object size in there
again, reducing the DRC header from 24 to 16 bytes.

Fixes #10459

view this post on Zulip Wasmtime GitHub notifications bot (Mar 24 2025 at 23:08):

fitzgen edited PR #10463:

Depends on https://github.com/bytecodealliance/wasmtime/pull/10462

We were not considering that when a type T is a subtype of type U, that all of
the fields that T and U share must be at the same offsets. Our
reordering/packing algorithm did not consider this. So we stop reordering struct
fields for now.

This also means that we cannot guarantee that all GC refs in an object are
contiguously packed anymore. Instead, we maintain a map keyed by
VMSharedTypeIndex, whose value is all the offsets of outgoing GC
references for instances of that type. Tracing looks up the current object's
type in this map, and then extracts each of the GC edges.

This new tracing scheme means that we no longer need to store the number of
outgoing edges in the GC object header, and can put the object size in there
again, reducing the DRC header from 24 to 16 bytes.

Fixes #10459

view this post on Zulip Wasmtime GitHub notifications bot (Mar 24 2025 at 23:29):

fitzgen updated PR #10463.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 00:48):

fitzgen updated PR #10463.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 01:03):

fitzgen updated PR #10463.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 01:04):

github-actions[bot] commented on PR #10463:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "wasmtime:api", "wasmtime:ref-types"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 10:38):

vouillon submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 10:38):

vouillon created PR review comment:

This assertion fails with the following test module.

(module
 (type $array (array (mut i64)))
 (export "_start" (func $f))
 (func $f
  (drop (array.new_fixed $array 0)))
)

We get this error:

assertion `left == right` failed
  left: 24
 right: 20

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 14:52):

fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 14:52):

fitzgen created PR review comment:

Thanks, this assertion should have been gated on the elements being GC refs

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 15:28):

fitzgen updated PR #10463.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 20:55):

dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 22:08):

fitzgen updated PR #10463.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 22:08):

fitzgen has enabled auto merge for PR #10463.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 25 2025 at 23:19):

fitzgen merged PR #10463.


Last updated: Apr 16 2025 at 19:03 UTC