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.

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

Decompress wasm runtime before validating #4182

Merged
merged 22 commits into from
May 12, 2023

Conversation

mnaamani
Copy link
Member

@mnaamani mnaamani commented Feb 16, 2023

Fixes #4122

Effect on bundle size, increased by ~600KB

New bundle size:

 | WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
 | Entrypoints:
 |   main (14.1 MiB)
 |       runtime~main.a482ee5c0a6b9c4e7e91.js
 |       main.6466ad456ae28e6c37a6.js

Old bundle size:

 | WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
 | Entrypoints:
 |   main (13.5 MiB)
 |       runtime~main.18b2bca8852a873cb1ab.js
 |       main.0fa1610cb597cee324a3.js

There is a separate issue with actually submitting the proposal, the "Create Proposal" button is not enabled.

Screenshot 2023-02-24 at 10 50 12 AM

@vercel
Copy link

vercel bot commented Feb 16, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
dao ✅ Ready (Inspect) Visit Preview May 12, 2023 0:03am
pioneer-2 ✅ Ready (Inspect) Visit Preview May 12, 2023 0:03am
pioneer-2-storybook ✅ Ready (Inspect) Visit Preview May 12, 2023 0:03am

@mnaamani
Copy link
Member Author

mnaamani commented Feb 24, 2023

Originally tried zstandard-wasm and @hpcc-js/wasm and the decompression seems to fail to work correctly. The decompressed file is always much larger than expected and fails to validate.

It seems both of those packages don't support "streaming decompression".

Finally used @oneidentity/zstd-js and that works correctly, but the package is ~67Mb.

Only @oneidentity/zstd-js/decompress is imported so hopefully the final webpack bundle size doesn't grow significantly.

const prefix = wasm.slice(0, 8)
const isCompressed = Buffer.compare(prefix, ZSTD_PREFIX) === 0
if (isCompressed) {
const { ZstdStream } = await ZstdInit()
Copy link
Member Author

Choose a reason for hiding this comment

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

I suspect this is not he best place to import or load the library, but I'm not an expert in React so perhaps it can be refactored by someone else, or suggest how/where to place initializing of the library and how to pass it down to this component.

Was also looking for a way to "unload" the library from memory (as I have seem some other wasm libraries do) but couldn't find it in the documentation.

Copy link
Member

Choose a reason for hiding this comment

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

I almost forgot about this! You're right it makes sense to move this lib out of the main bundle.

I chose to move it to a web worker in this commit (because it's already a common pattern in Pioneer) however I'm not sure where did the library actually went:
Screenshot from 2023-05-12 14-37-51

The main bundle lost the correct size, but I couldn't find any bundles which significantly grew. Maybe the lib was already a dependency in that chunk.

@mnaamani mnaamani marked this pull request as ready for review February 24, 2023 07:12
@thesan
Copy link
Member

thesan commented May 12, 2023

@mnaamani thank you for the WASM validation logic 🙏 I would have struggled with this !

@thesan thesan added builders-wg-code-review waiting-for-checks PR is reviewed and approved, but is waiting for checks to complete and removed builders-wg-code-review labels May 12, 2023
@thesan thesan merged commit f3c728f into Joystream:dev May 12, 2023
@thesan thesan removed the waiting-for-checks PR is reviewed and approved, but is waiting for checks to complete label May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WASM validation fails for compressed runtime blob
2 participants