Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use Custom Rust Targets for Shared Wasm Libraries #4683

Open
wants to merge 12 commits into
base: luc/upgrade-rts-dependencies
Choose a base branch
from
Open

Use Custom Rust Targets for Shared Wasm Libraries #4683

wants to merge 12 commits into from

Conversation

luc-blaeser
Copy link
Contributor

@luc-blaeser luc-blaeser commented Sep 3, 2024 •
edited
Loading

Simplify Motoko RTS building:

  • Use custom Rust targets for generating PIC-relocatable Wasm.
  • Get rid of emscripten or explicit compilation via LLVM IR.
  • Get rid of musl (until now, it was only used for classical persistence).

Note: Requires bug fix #4685 in the Motoko linker.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 6, 2024

Comparing from a6d1305 to 0c4a9dd:
In terms of gas, 5 tests regressed and the mean change is +0.0%.
In terms of size, 5 tests improved and the mean change is -41.0%.

@luc-blaeser luc-blaeser marked this pull request as ready for review September 9, 2024 08:09
@luc-blaeser luc-blaeser self-assigned this Sep 9, 2024
@luc-blaeser luc-blaeser changed the title Experiment: Use Custom Rust Targets for Shared Wasm Libraries Use Custom Rust Targets for Shared Wasm Libraries Sep 9, 2024
@luc-blaeser luc-blaeser added the dependencies Pull requests that update a dependency file label Sep 9, 2024
crusso
crusso approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@crusso crusso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't pretend to know what is going one but this seems like a nice cleanup removing musl from the classical compiler. Shame the instruction counts seem to go up.

test/bench/ok/bignum.drun-run.ok
ingress Completed: Reply: 0x4449444c0000
debug.print: {cycles = 105_771_809; size = +1_817_872}
debug.print: {cycles = 111_509_014; size = +1_817_872}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why this has increased so much?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I need to analyze why.

@luc-blaeser luc-blaeser mentioned this pull request Sep 9, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crusso crusso crusso approved these changes

@ggreif ggreif Awaiting requested review from ggreif

Assignees

@luc-blaeser luc-blaeser

Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@luc-blaeser @crusso