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

Treeshaking not working with maci library packages #781

Closed
yuetloo opened this issue Oct 26, 2023 · 8 comments
Closed

Treeshaking not working with maci library packages #781

yuetloo opened this issue Oct 26, 2023 · 8 comments
Assignees

Comments

@yuetloo
Copy link
Contributor

yuetloo commented Oct 26, 2023

We noticed that by importing just 1 function (genTallyResultCommitment) from the maci-core library, our web app size would double. Duplicating that function (as opposed to importing from maci-core) would bring the web app size back down.

Here are the sizes of our web app:

(import from maci-core)
7,567.22 kB │ gzip: 3,538.62 kB

(duplicate function locally)
3,669.37 kB │ gzip: 1,726.62 kB

I tried different rollup treeshaking options on our vite.config file, without success.
I tried adding "sideEffects: false" manually in the maci-core package in the local node_modules folder and still not working.

While working on using the snarkjs API, I stumbled onto this issue (iden3/snarkjs#152) which shows that ffjavascript library that maci-crypto depends on has side effects that might be causing treeshaking to not work.

For optimization, ffjavascript uses a global (globalThis variable) to pass back a handle for the consumer to terminate the threads.

Note that maci-crypto only uses the utils functions in (ffjavascript)[https://github.com/privacy-scaling-explorations/maci/blob/dev/crypto/ts/index.ts#L9), may be duplicating those functions instead of require('ffjavascript) could reduce the bundle size of app?

Also, moving the genTallyResultCommitment from maci-core to maci-crypto may help as we only need to import maci-crypto.

We use all the hash and encrypt/decrypt functions in maci-crypto to generate key and encrypt/decrypt messages on our web app.

Appreciate any suggestion on what to do to enable treeshaking and reduce our web app size.

@ctrlc03 ctrlc03 self-assigned this Oct 27, 2023
@ctrlc03
Copy link
Collaborator

ctrlc03 commented Oct 27, 2023

We noticed that by importing just 1 function (genTallyResultCommitment) from the maci-core library, our web app size would double. Duplicating that function (as opposed to importing from maci-core) would bring the web app size back down.

Here are the sizes of our web app:

(import from maci-core) 7,567.22 kB │ gzip: 3,538.62 kB

(duplicate function locally) 3,669.37 kB │ gzip: 1,726.62 kB

I tried different rollup treeshaking options on our vite.config file, without success. I tried adding "sideEffects: false" manually in the maci-core package in the local node_modules folder and still not working.

While working on using the snarkjs API, I stumbled onto this issue (iden3/snarkjs#152) which shows that ffjavascript library that maci-crypto depends on has side effects that might be causing treeshaking to not work.

For optimization, ffjavascript uses a global (globalThis variable) to pass back a handle for the consumer to terminate the threads.

Note that maci-crypto only uses the utils functions in (ffjavascript)[https://github.com/privacy-scaling-explorations/maci/blob/dev/crypto/ts/index.ts#L9), may be duplicating those functions instead of require('ffjavascript) could reduce the bundle size of app?

Also, moving the genTallyResultCommitment from maci-core to maci-crypto may help as we only need to import maci-crypto.

We use all the hash and encrypt/decrypt functions in maci-crypto to generate key and encrypt/decrypt messages on our web app.

Appreciate any suggestion on what to do to enable treeshaking and reduce our web app size.

I'm gonna try a few things and see what we can do to address this. For genTallyResultCommitment, I see no problem in moving this, it could easily be named smth more general like hashTree and kept in crypto. ps. I'm refactoring all of the crypto package here #749

@ctrlc03 ctrlc03 added this to MACI Oct 30, 2023
@ctrlc03 ctrlc03 moved this to Todo in MACI Oct 30, 2023
@ctrlc03 ctrlc03 added this to the Maci Refactoring milestone Oct 31, 2023
@samajammin
Copy link
Member

Wow thanks for reporting this @yuetloo (& for the investigation)!

@ctrlc03 ctrlc03 removed this from the MACI v1.1.1 refactor milestone Nov 10, 2023
@ctrlc03 ctrlc03 added this to the MACI v1.1.1 refactor milestone Nov 28, 2023
@samajammin samajammin assigned 0xmad and unassigned ctrlc03 Dec 12, 2023
@0xmad 0xmad moved this from Todo to In Progress in MACI Jan 8, 2024
@yuetloo
Copy link
Contributor Author

yuetloo commented Jan 9, 2024

(using maci npm package: 0.0.0-ci.45668db)
5,077.19 kB │ gzip: 2,400.17 kB

(previous version of maci v1)
3,662.97 kB │ gzip: 1,725.67 kB

@0xmad @ctrlc03 it looks like something else in the current MACI dev branch has increased the bundle size, not sure what it is because there's a lot of changes in the current MACI dev branch since clrfund cloned the MACI v1 branch.

However, moving genTallyResultCommitment to the crypto package did reduce the bundle size from 7,567.22 kB │ gzip: 3,538.62 kB to currently at 5,077.19 kB │ gzip: 2,400.17 kB

@0xmad
Copy link
Collaborator

0xmad commented Jan 9, 2024

I think it's related to tests and source folders. We need to build bundle with separate tsconfig instead of using only one which includes all the ts code needed for development.

@0xmad
Copy link
Collaborator

0xmad commented Jan 9, 2024

@yuetloo new version is published.
New: https://www.npmjs.com/package/maci-crypto/v/0.0.0-ci.8cb022e - 190KB (Unpacked size)
Old: https://www.npmjs.com/package/maci-crypto/v/0.0.0-ci.d8c808e - 305KB (Unpacked size)

@samajammin
Copy link
Member

@0xmad nice!

So did #974 solve this issue? Or is there more to be done here?

@0xmad
Copy link
Collaborator

0xmad commented Jan 11, 2024

@samajammin it should. If @yuetloo is ok with this fix we can close this issue.

@0xmad
Copy link
Collaborator

0xmad commented Jan 17, 2024

@yuetloo feel free to reopen it if the fixes don't work for you.

@0xmad 0xmad closed this as completed Jan 17, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in MACI Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

4 participants