Skip resource-borrow scope tracking for borrow-free signatures#13888
Skip resource-borrow scope tracking for borrow-free signatures#13888tschneidereit wants to merge 2 commits into
Conversation
tschneidereit
added
the
wasmtime
Precompute `TypeFunc::contains_borrow` at compile time and use it in the hostcall entrypoint to skip `CallContext` scope push/pop and `validate_scope_exit` when lending is statically impossible. Relative to bytecodealliance#13887, this reduces call overhead by about 29% for sync calls, and about 9% for async calls (both measured in a Linux VM on an M5 Max MBP): sync calls go from about 65ns to about 46ns, immediately ready async calls from 117ns to 106ns. I'm not entirely sure why the win is so much less for async calls, but there are more wins in future commits.
Extends skipping scope resolution introduced in the previous commit to also cover the LiftContext used in sync calls. Plus, inline a bunch of single-instantiation generics. Relative to the previous commit, this reduces call overhead by another 31% for sync calls (46ns to 32ns), with async calls unchanged (both measured in a Linux VM on an M5 Max MBP).
d37fb2f to
abc6aab
Compare
github-actions
Bot
added
the
wasmtime:api
|
I've self-requested a review of myself here, this is something I'll want to sit down and think about for a bit as how it relates to the various bits of state-management this is skipping. |
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Hm ok personally I'm not quite comfortable landing this yet. I'd like to better understand what's being optimized here and how this is achieving that. It looks like this new want_scope: bool option is serving double-duty where in concurrency_support-disabled builds it's avoiding a push/pop on a stack, and in all builds it's avoiding an invocation of store.current_scope_id()?. I've no doubt that avoiding the latter would likely yield a speed boost, but the former I find confusing to follow and would prefer to not add (and given that it seems to only affect concurrency_support-disabled builds it might be fine to remove).
I'm in general wary of making the state machine/states/etc here more complex. Async is already complex enough and every new variant of "if X do Y else Z" explodes the state space 2x that needs to be considered. If this is isolated to just a small piece of code I think that's fine, but this is pretty pervasive affecting core data structures.
This is why I'd like to better understand what this is optimizing and how it's optimizing it. I don't have a great handle on that as-is and am unable to conclude if this is the best way to avoid the overhead or not.
3 participants
Combined, the two contained commits improve call overhead for sync calls without borrows byt about 50%, from 65ns to 32ns, and immediately ready async calls without borrows by about 9%, from 117ns to 106. All numbers relative to #13887.