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

Reduce static libraries sizes #37

Closed
1 task
richard-ramos opened this issue Aug 29, 2022 · 17 comments
Closed
1 task

Reduce static libraries sizes #37

richard-ramos opened this issue Aug 29, 2022 · 17 comments
Assignees
Labels
track:zerokit Zerokit track (Applied ZK/Explorations)

Comments

@richard-ramos
Copy link
Member

richard-ramos commented Aug 29, 2022

Problem

While attempting to implement go-rln-zerokit, I noticed that the resulting static libraries size are very large compared to kilic/rln:

  • kilic/rln
    ⁃ aarch64-apple-darwin: 21.9MB
    ⁃ x86_64-apple-darwin: 22.6MB
    ⁃ aarch64-apple-ios: 21.8MB
    ⁃ x86_64-apple-ios: 22.6MB
    ⁃ universal (supporting aarch64-apple-ios,x86_64-apple-ios): 44.4MB
  • vacp2p/zerokit
    ⁃ aarch64-apple-darwin: 57.7MB
    ⁃ x86_64-apple-darwin: 59.5MB
    ⁃ aarch64-apple-ios: 152MB
    ⁃ x86_64-apple-ios: 60.5MB
    ⁃ universal (supporting aarch64-apple-ios,x86_64-apple-ios): 212.5MB

While this is not an issue for desktop architecture, it is certainly not nice to have an app size increase due to the libraries it uses (also, I find it interesting that aarch64-apple-ios is 152MB, compared to the 60.5MB of x86_64-apple-ios).

I used the following commands to generate the static libs.

rustup target add aarch64-apple-ios x86_64-apple-ios x86_64-apple-darwin aarch64-apple-darwin
cargo build --release --target=x86_64-apple-darwin --lib
cargo build --release --target=aarch64-apple-darwin --lib
cargo lipo --release

Acceptance criteria

  • Reduce as much as possible library size by checking/refactoring dependencies and the module
@fryorcraken
Copy link
Contributor

This is likely to be an issue for integration of RLN in js-waku (would be good to update this issue with wasm output once we have it). In the browser, a good size library is under 1MB.

Looking at the current code organisation, I see a single crate for RLN. I am not totally familiar with wasm compilation. I don't know if you could only compile "some" of the API but I do know that you usually have to create a wasm API that then use the Rust API (similar to react native).

When starting the JS/wasm code, it would be good to split the wasm API in scoped functionality: generate credentials, generate proof, verify proof so that it is possible to generate a wasm blob that can generate credentials and generate proof but not unnecessary "verify proof" code. We probably want to go toward a flexible road so that an app developer can decide whether to:

  • have one app to generate credentials and another to generate proof (the generate credentials apps can be downloaded once but then never again) and hence have 2 different wasm blob for issue
  • have one app that does both, hence, have 1 wasm blob that includes both features

Depending on how big the wasm blob is (ie, unusable in the browser) then this issue may need to be prioritised before the optimisation phase.

Note: added to RAID for Waku Product.

@s1fr0
Copy link
Contributor

s1fr0 commented Sep 6, 2022

WIP: I was able to reduce the size, e.g. for x86_64-apple-darwin from ~60MB to 42MB by just using a light version of ark-circom without the ethers-core/ethers-rs heavy dependencies (not relevant for us). Based on some other local tests I've done, I think that by removing dependencies used for tests and by integrating all those small parts we use from heaviest dependencies, we might be able to successfully drop library size to ~30MB and hopefully less.

a good size library is under 1MB.

do you expect the whole RLN WASM/*.a to be around this size? I think it is a quite unpractical target at the moment, unless we do lots of surgical copy/paste from dependencies (the arkworks ecosystem is quite big, and we probably lose a decent amount of code versatility/manageability if we go with something like that)

@richard-ramos
Copy link
Member Author

These are great news! Awesome job, @s1fr0!

@fryorcraken
Copy link
Contributor

a good size library is under 1MB.

do you expect the whole RLN WASM/*.a to be around this size? I think it is a quite unpractical target at the moment, unless we do lots of surgical copy/paste from dependencies (the arkworks ecosystem is quite big, and we probably lose a decent amount of code versatility/manageability if we go with something like that)

Ultimately yes, but this would go under the optimization effort for an optimal browser experience. Not too worry about right now.

@s1fr0 s1fr0 self-assigned this Sep 7, 2022
@s1fr0
Copy link
Contributor

s1fr0 commented Sep 7, 2022

I self-assigned this issue to me, since I'm working on reducing more libraries sizes.

@s1fr0 s1fr0 added the track:zerokit Zerokit track (Applied ZK/Explorations) label Sep 7, 2022
@s1fr0 s1fr0 mentioned this issue Sep 7, 2022
10 tasks
@s1fr0 s1fr0 changed the title static libraries are too large Reduce static libraries sizes Sep 7, 2022
@gakonst
Copy link

gakonst commented Sep 20, 2022

Let us know if we can trim any deps on the ethers side, or introduce any features which let you further customize what you pull in!

@tyshko-rostyslav
Copy link
Contributor

@tyshko-rostyslav
Copy link
Contributor

@tyshko-rostyslav
Copy link
Contributor

@richard-ramos Could you please, take a look at this diff diff_with_our_fork to make sure that all the changes here are needed (especially the [lib] part), so that I could create a PR to original ark-circom repo, get it merged and remove an another instance of ark-circom from our binary, radically reducing its size ?

@gakonst
Copy link

gakonst commented Mar 15, 2023

Supportive of all changes that can make Ark Circom less heavy, appreciate your guys' help here.

@tyshko-rostyslav
Copy link
Contributor

@tyshko-rostyslav
Copy link
Contributor

@rahulghangas @rymnc what do you think of this issue #135 regarding the overall context?

@rahulghangas
Copy link
Contributor

As long as no compilation issues come up, sounds good to me

@richard-ramos
Copy link
Member Author

@richard-ramos Could you please, take a look at this diff diff_with_our_fork to make sure that all the changes here are needed (especially the [lib] part), so that I could create a PR to original ark-circom repo, get it merged and remove an another instance of ark-circom from our binary, radically reducing its size ?

The [lib] part i think it was me doing some experiments and it's not really needed as we're not using ark-circom as a dynamic library. The other changes (default-features = false) are/were necessary, because the parallels feature didn't let me use zerokit in a wasm32 environment, due to the problem described here #55

@tyshko-rostyslav
Copy link
Contributor

As we are now using wasm-strip #137 and got rid of our version of ark-circom #132 I think for now there are no ways to reduce size even more. I propose to close this issue. What do you think @rahulghangas ?

@rahulghangas
Copy link
Contributor

I agree that it can be closed for now, @rymnc thoughts?

@rymnc
Copy link
Contributor

rymnc commented Apr 5, 2023

Agreed. there may be some further optimizations we can do over time, but let's close this and open a specific issue later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
track:zerokit Zerokit track (Applied ZK/Explorations)
Projects
Status: Done
Development

No branches or pull requests

7 participants