Stream: git-wasmtime

Topic: wasmtime / PR #7927 winch: Optimize calls

Wasmtime GitHub notifications bot (Feb 13 2024 at 17:23):

saulecabrera requested elliottt for a review on PR #7927.

Wasmtime GitHub notifications bot (Feb 13 2024 at 17:23):

saulecabrera opened PR #7927 from saulecabrera:some-optimizations to bytecodealliance:main:

This commit introduces several optimizations to speed up the compilation of function calls:

• Keep track of previously resolved function signatures for local or imported callees to avoid computing the ABISig on every function call.
• Keep track of previously resolved type signatures for indirect calls to avoid computing the ABISig on every function call.
• Refactor CallKnown and CallUnknown instructions to make the BoxCallInfo fiel in the struction optional. Prior to this change, from Winch's perspective each call lowering involved a heap allocation, using the default values for BoxCallInfo, which in the end are not used by Winch.
• Switch Winch's internal Stack to use a SmallVec rather than a Vec. Many of the operations involving builtin function calls require inserting elements at arbitrary offsets in the stack and using a SmallVec makes this process more efficient.

With the changes mentioned above, I observed ~30% improvement in compilation times for modules that are call-heavy.

<!--
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 ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Feb 13 2024 at 17:23):

saulecabrera requested wasmtime-compiler-reviewers for a review on PR #7927.

Wasmtime GitHub notifications bot (Feb 13 2024 at 17:31):

saulecabrera submitted PR review.

Wasmtime GitHub notifications bot (Feb 13 2024 at 17:31):

saulecabrera created PR review comment:

Some thoughts that I had while changing this:

• In case there are no objections or downsides to making BoxCallInfo optional, would it be preferred to expect that the info is present in this path?
• Another alternative that I considered in case that making BoxCallInfo optional is not desired, would be to create another instruction without BoxCallInfo, which would be only consumed by Winch, at the cost of some code duplication.

Wasmtime GitHub notifications bot (Feb 13 2024 at 17:35):

saulecabrera updated PR #7927.

Wasmtime GitHub notifications bot (Feb 13 2024 at 17:36):

saulecabrera updated PR #7927.

Wasmtime GitHub notifications bot (Feb 13 2024 at 17:37):

saulecabrera edited PR #7927:

This commit introduces several optimizations to speed up the compilation of function calls:

• Keep track of previously resolved function signatures for local or imported callees to avoid computing the ABISig on every function call.
• Keep track of previously resolved type signatures for indirect calls to avoid computing the ABISig on every function call.
• Refactor CallKnown and CallUnknown instructions to make the BoxCallInfo field in the struction optional. Prior to this change, from Winch's perspective each call lowering involved a heap allocation, using the default values for BoxCallInfo, which in the end are not used by Winch.
• Switch Winch's internal Stack to use a SmallVec rather than a Vec. Many of the operations involving builtin function calls require inserting elements at arbitrary offsets in the stack and using a SmallVec makes this process more efficient.

With the changes mentioned above, I observed ~30% improvement in compilation times for modules that are call-heavy.

<!--
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 ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Feb 13 2024 at 17:37):

saulecabrera edited PR #7927:

This commit introduces several optimizations to speed up the compilation of function calls:

• Keep track of previously resolved function signatures for local or imported callees to avoid computing the ABISig on every function call.
• Keep track of previously resolved type signatures for indirect calls to avoid computing the ABISig on every function call.
• Refactor CallKnown and CallUnknown instructions to make the BoxCallInfo field in the struct optional. Prior to this change, from Winch's perspective each call lowering involved a heap allocation, using the default values for BoxCallInfo, which in the end are not used by Winch.
• Switch Winch's internal Stack to use a SmallVec rather than a Vec. Many of the operations involving builtin function calls require inserting elements at arbitrary offsets in the stack and using a SmallVec makes this process more efficient.

With the changes mentioned above, I observed ~30% improvement in compilation times for modules that are call-heavy.

<!--
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 ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Feb 13 2024 at 17:44):

saulecabrera updated PR #7927.

Wasmtime GitHub notifications bot (Feb 13 2024 at 20:44):

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

Subscribe to Label Action

cc @cfallin, @fitzgen, @saulecabrera

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:x64", "isle", "winch"

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

• cfallin: isle
• fitzgen: isle
• saulecabrera: winch

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

Learn more.
</details>

Wasmtime GitHub notifications bot (Feb 13 2024 at 23:23):

elliottt submitted PR review:

This seems good to me!

Wasmtime GitHub notifications bot (Feb 13 2024 at 23:23):

elliottt submitted PR review:

This seems good to me!

Wasmtime GitHub notifications bot (Feb 13 2024 at 23:23):

elliottt created PR review comment:

This doesn't seem like a problem to me, and using .expect() would help to convey that it must be present for cranelift's purposes. Also, this change doesn't alter the size of the MInst enum, so we're not paying for it in a hidden way.

Would you mind commenting on the definition of BoxCallInfo in isle.rs that it's optional only for winch?

Wasmtime GitHub notifications bot (Feb 13 2024 at 23:23):

elliottt created PR review comment:

impl Callee<'_> {
    /// Returns the [ABISig] of the [Callee].
    pub(crate) fn sig(&self) -> &ABISig {

Wasmtime GitHub notifications bot (Feb 13 2024 at 23:23):

elliottt created PR review comment:

        self.masm.float_round(

Wasmtime GitHub notifications bot (Feb 14 2024 at 11:32):

saulecabrera updated PR #7927.

Wasmtime GitHub notifications bot (Feb 14 2024 at 11:33):

saulecabrera submitted PR review.

Wasmtime GitHub notifications bot (Feb 14 2024 at 11:33):

saulecabrera created PR review comment:

Thanks for the suggestion! Added a comment.

Wasmtime GitHub notifications bot (Feb 14 2024 at 12:38):

saulecabrera merged PR #7927.

Last updated: Dec 23 2024 at 12:05 UTC