Stream: git-wasmtime

Topic: wasmtime / PR #14997 component: Avoid redundant type chec...

Wasmtime GitHub notifications bot (Oct 26 2026 at 18:08):

adamrk requested pchickey for a review on PR #14997.

Wasmtime GitHub notifications bot (Oct 26 2026 at 18:08):

adamrk opened PR #14997 from adamrk:abk/resource-table-delete to bytecodealliance:main:

delete previously checked is::<Tombstone>() on every deleted entry, even though the Tombstones are only used for debugging purposes. This added an extra downcast which should be elided in release builds. Overall this produces an improvement of about 4.5% in the p3_http_proxy benchmark.

Note that the improvements here seem to be a bit sensitive to inlining, so I added some explicit inlining which helped improve the benchmark results. A comparison against main when both use codegen-units=1 still shows an improvement in the p3_http_proxy benchmark, so I think the top line 4.5% improvement is coming from a mix of "real wins" and better inlining.

Benchmark results:

Main (ffff2e2db24a291f02429f123aba1631fa8eea67):

wasi-http/p3_http_proxy time:   [18.444 µs 18.465 µs 18.491 µs]
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) high mild
  3 (3.00%) high severe

wasi-http/p3-api/request.new
                        time:   [1.4723 µs 1.4745 µs 1.4772 µs]
Found 6 outliers among 100 measurements (6.00%)
  2 (2.00%) low mild
  3 (3.00%) high mild
  1 (1.00%) high severe

wasi-http/p3-api/response.consume-body
                        time:   [2.0563 µs 2.0701 µs 2.0824 µs]

wasi-http/p3-api/client.send
                        time:   [2.8368 µs 2.8734 µs 2.9408 µs]
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) high mild
  2 (2.00%) high severe

Branch (9d705daf080401a631fe39e56af6ab55d2aa4f7f):

wasi-http/p3_http_proxy time:   [17.591 µs 17.603 µs 17.621 µs]
                        change: [−4.8620% −4.6660% −4.4654%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  2 (2.00%) high mild
  5 (5.00%) high severe

wasi-http/p3-api/request.new
                        time:   [1.4472 µs 1.4494 µs 1.4519 µs]
                        change: [−1.9608% −1.5173% −1.0651%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) low mild
  1 (1.00%) high mild
  2 (2.00%) high severe

wasi-http/p3-api/response.consume-body
                        time:   [2.0912 µs 2.1192 µs 2.1609 µs]
                        change: [+0.3970% +1.4221% +2.5705%] (p = 0.01 < 0.05)
                        Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe

wasi-http/p3-api/client.send
                        time:   [2.8608 µs 2.8639 µs 2.8668 µs]
                        change: [−0.9810% −0.0330% +0.6530%] (p = 0.94 > 0.05)
                        No change in performance detected.
Found 4 outliers among 100 measurements (4.00%)
  3 (3.00%) high mild
  1 (1.00%) high severe

Main (ffff2e2db24a291f02429f123aba1631fa8eea67) with codegen-units=1:

wasi-http/p3_http_proxy time:   [16.848 µs 16.867 µs 16.894 µs]
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) high mild
  3 (3.00%) high severe

wasi-http/p3-api/request.new
                        time:   [1.3782 µs 1.3851 µs 1.3974 µs]
Found 4 outliers among 100 measurements (4.00%)
  2 (2.00%) high mild
  2 (2.00%) high severe

wasi-http/p3-api/response.consume-body
                        time:   [1.9108 µs 1.9358 µs 1.9779 µs]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe

wasi-http/p3-api/client.send
                        time:   [2.6893 µs 2.6927 µs 2.6959 µs]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe

Branch (9d705daf080401a631fe39e56af6ab55d2aa4f7f) with codegen-units=1:

wasi-http/p3_http_proxy time:   [16.463 µs 16.490 µs 16.526 µs]
                        change: [−2.4924% −2.2750% −2.0643%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  3 (3.00%) high mild
  5 (5.00%) high severe

wasi-http/p3-api/request.new
                        time:   [1.3414 µs 1.3437 µs 1.3464 µs]
                        change: [−3.5348% −2.8602% −2.2768%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  3 (3.00%) high mild
  2 (2.00%) high severe

wasi-http/p3-api/response.consume-body
                        time:   [1.8812 µs 1.8930 µs 1.9082 µs]
                        change: [−2.6282% −1.4599% −0.2754%] (p = 0.01 < 0.05)
                        Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high severe

wasi-http/p3-api/client.send
                        time:   [2.6486 µs 2.6519 µs 2.6551 µs]
                        change: [−1.7366% −1.3272% −0.7781%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high severe

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please review the Bytecode Alliance's AI tool usage policy at
https://github.com/bytecodealliance/governance/blob/main/AI_TOOL_POLICY.md

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Oct 26 2026 at 18:08):

adamrk removed the review request for wasmtime-core-reviewers from PR #14997.

Wasmtime GitHub notifications bot (Oct 26 2026 at 18:08):

adamrk requested wasmtime-core-reviewers for a review on PR #14997.

Wasmtime GitHub notifications bot (Oct 26 2026 at 18:46):

:thumbs_up: pchickey submitted PR review:

Awesome nice job

Wasmtime GitHub notifications bot (Oct 26 2026 at 20:48):

github-actions[bot] added the label wasmtime:api on PR #14997.

Wasmtime GitHub notifications bot (Oct 27 2026 at 14:31):

adamrk updated PR #14997.

Wasmtime GitHub notifications bot (Oct 27 2026 at 14:33):

adamrk commented on PR #14997:

There was a test failing because it mixed deleting with and without tombstones which isn't properly handled with this change (the whole point is that we don't want to check for tombstones when not debugging). So I moved the type parameter to be on ResourceTable itself which means it's no longer possible to mix the two uses within the same table (and modified the test to only use the debug version).

I also reran the benchmarks to check that we still see the same improvement.

Wasmtime GitHub notifications bot (Oct 27 2026 at 14:34):

adamrk requested pchickey for a review on PR #14997.

Wasmtime GitHub notifications bot (Oct 27 2026 at 14:36):

alexcrichton commented on PR #14997:

FWIW this was originally just intended as a debugging utility and aid, and IIRC the debug: bool parmeter is only there to aid with testing the implementation should the flag ever turn to true. In that sense I don't think it's worth going too far out of our way to keep supporting this -- if it's a perf problem I'd say we could just delete the support outright entirely since it's all off-by-default anyway. Dealing with sealed traits, impls, default type parameters, etc, to me feels a bit heavyweight relative to what it's trying to do.

Or, alternatively, for the perf problems you were seeing, is that all because debug: bool wasn't inlined to get const-propagated away? If that's the case then #[inline] could help, but I'd again consider keeping a debug-only utility like this a nice-to-have and would prioritize jettisoning it if it's causing problems.

Wasmtime GitHub notifications bot (Oct 27 2026 at 14:49):

alexcrichton commented on PR #14997:

(feel free to land this though in the meantime, I'm mostly just musing that as a debug-only feature it's fine to just jettison it)

Wasmtime GitHub notifications bot (Oct 27 2026 at 18:22):

adamrk commented on PR #14997:

Makes sense - I'll merge this as is and next time ask if anyone is actually still using the thing...

Or, alternatively, for the perf problems you were seeing, is that all because debug: bool wasn't inlined to get const-propagated away?

Pure inlining wouldn't have been sufficient before because eliding the check for a Tombstone would require knowing that no Tombstones exist (i.e. that delete_maybe_debug was _never_ called with debug: true), not just that debug is a const false in the current call.

Wasmtime GitHub notifications bot (Oct 27 2026 at 18:22):

adamrk added PR #14997 component: Avoid redundant type check and entry move in ResourceTable::delete to the merge queue

Wasmtime GitHub notifications bot (Oct 27 2026 at 18:45):

:check: adamrk merged PR #14997.

Wasmtime GitHub notifications bot (Oct 27 2026 at 18:45):

adamrk removed PR #14997 component: Avoid redundant type check and entry move in ResourceTable::delete from the merge queue

Last updated: Oct 31 2026 at 23:03 UTC