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

0-copy module deserialization #4139

Merged
merged 16 commits into from
Aug 17, 2023
Merged

0-copy module deserialization #4139

merged 16 commits into from
Aug 17, 2023

Conversation

Arshia001
Copy link
Member

Closes #3882. This change lets modules be loaded directly from archived structs by adding traits that are implemented by both plain and archived versions of structs used in modules.

@Arshia001 Arshia001 requested a review from syrusakbary as a code owner August 9, 2023 11:24
@Arshia001 Arshia001 marked this pull request as draft August 9, 2023 11:24
syrusakbary
syrusakbary approved these changes Aug 10, 2023
View reviewed changes
Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

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

PR looks great other than for the few comments

@Arshia001 Arshia001 marked this pull request as ready for review August 10, 2023 05:10
theduke
theduke requested changes Aug 10, 2023
View reviewed changes
Copy link
Contributor

@theduke theduke left a comment

Choose a reason for hiding this comment

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

Great work!

Mostly some nitpicks, and a request for adding some tests.

lib/api/src/engine.rs Outdated Show resolved Hide resolved
lib/api/src/engine.rs Outdated Show resolved Hide resolved
lib/compiler/src/artifact_builders/artifact_builder.rs Outdated Show resolved Hide resolved
lib/compiler/src/engine/link.rs Show resolved Hide resolved
lib/types/src/compilation/function.rs Show resolved Hide resolved
lib/compiler/src/engine/artifact.rs Show resolved Hide resolved
lib/compiler/src/engine/artifact.rs Show resolved Hide resolved
@Arshia001
Copy link
Member Author

According to my measurements, on average this change this much reduction in module load times:

  • 40% for cowsay
  • 50% for rustpython
  • 47% for php

@Arshia001 Arshia001 enabled auto-merge August 17, 2023 16:00
@theduke theduke disabled auto-merge August 17, 2023 16:19
@theduke theduke added 🎉 enhancement New feature! 📦 lib-compiler About wasmer-compiler labels Aug 17, 2023
@theduke
Copy link
Contributor

theduke commented Aug 17, 2023

This is approved, but we should wait with the merge until 4.1.2 is out, since this is a pretty big change and we want to give it some time to bake on master.

@Arshia001 Arshia001 merged commit 55cfdc9 into master Aug 17, 2023
@Arshia001 Arshia001 deleted the feat/0-copy-archive-deserialization branch August 17, 2023 17:41
@ptitSeb
Copy link
Contributor

ptitSeb commented Aug 17, 2023

Why is this merged!!!!

@syrusakbary syrusakbary restored the feat/0-copy-archive-deserialization branch August 17, 2023 19:01
@syrusakbary syrusakbary mentioned this pull request Aug 17, 2023
Merged
@Arshia001
Copy link
Member Author

@ptitSeb sorry, I missed @theduke's laat comment. My bad.

@theduke theduke mentioned this pull request Aug 25, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theduke theduke theduke approved these changes

@syrusakbary syrusakbary syrusakbary approved these changes

@Michael-F-Bryan Michael-F-Bryan Awaiting requested review from Michael-F-Bryan

Assignees
No one assigned
Labels
🎉 enhancement New feature! 📦 lib-compiler About wasmer-compiler
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

mmap Support for Artifacts / deserialized Modules
4 participants
@Arshia001 @theduke @ptitSeb @syrusakbary