Stream: git-wasmtime

Topic: wasmtime / PR #2394 Remove size-of-struct asserts that br...


view this post on Zulip Wasmtime GitHub notifications bot (Nov 11 2020 at 21:50):

cfallin opened PR #2394 from no-size-asserts to main:

The asserts on the sizes of the VCode constant-table data structures
introduced in PR #2328 are dependent on the size of data structures such
as HashMap in the standard library, which can change. In particular,
on Rust 1.46 (which is not current, but could be e.g. pinned by a
project using Cranelift), it appears that these asserts fail. We
shouldn't depend on stdlib internals; IMHO the asserts on our own struct
sizes are enough to catch accidental size blowups.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Nov 11 2020 at 21:50):

cfallin requested abrown for a review on PR #2394.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 11 2020 at 21:56):

cfallin updated PR #2394 from no-size-asserts to main:

The asserts on the sizes of the VCode constant-table data structures
introduced in PR #2328 are dependent on the size of data structures such
as HashMap in the standard library, which can change. In particular,
on Rust 1.46 (which is not current, but could be e.g. pinned by a
project using Cranelift), it appears that these asserts fail. We
shouldn't depend on stdlib internals; IMHO the asserts on our own struct
sizes are enough to catch accidental size blowups.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Nov 12 2020 at 00:54):

abrown submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 12 2020 at 00:54):

abrown submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 12 2020 at 00:54):

abrown created PR Review Comment:

Here's a suggestion for documentation to replace this:

// TODO The VCodeConstants structure's memory size could be further optimized.
// With certain versions of Rust, each `HashMap` in `VCodeConstants` occupied at
// least 48 bytes, making an empty `VCodeConstants` cost 120 bytes.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 12 2020 at 01:09):

cfallin updated PR #2394 from no-size-asserts to main:

The asserts on the sizes of the VCode constant-table data structures
introduced in PR #2328 are dependent on the size of data structures such
as HashMap in the standard library, which can change. In particular,
on Rust 1.46 (which is not current, but could be e.g. pinned by a
project using Cranelift), it appears that these asserts fail. We
shouldn't depend on stdlib internals; IMHO the asserts on our own struct
sizes are enough to catch accidental size blowups.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Nov 12 2020 at 01:09):

cfallin submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 12 2020 at 01:09):

cfallin created PR Review Comment:

Good idea, thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Nov 12 2020 at 01:13):

cfallin updated PR #2394 from no-size-asserts to main:

The asserts on the sizes of the VCode constant-table data structures
introduced in PR #2328 are dependent on the size of data structures such
as HashMap in the standard library, which can change. In particular,
on Rust 1.46 (which is not current, but could be e.g. pinned by a
project using Cranelift), it appears that these asserts fail. We
shouldn't depend on stdlib internals; IMHO the asserts on our own struct
sizes are enough to catch accidental size blowups.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Nov 12 2020 at 02:04):

cfallin merged PR #2394.


Last updated: Dec 23 2024 at 12:05 UTC