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

feat: Reduce bundle sizes, remove polyfills #10877

Merged
merged 18 commits into from
Dec 20, 2024
Merged

Conversation

Thunkar
Copy link
Contributor

@Thunkar Thunkar commented Dec 19, 2024

Cleans up debug metadata for contract artifacts in protocol-contracts and noir-protocol-circuit-types, adding an optional build task to keep them. This heavily impacts the amount of data that we import in other parts of the code.

Also cleans up some testing imports that forced us to use weird vite plugins to conditionally delete blocks of code.

Starts the very much needed code splitting in circuits.js isolating everything that has to do with blobs and part of the rollup-only types.

Finally, this allows us to create a minimal vite.config.ts file that only polyfills path, buffer and process even with PXE proving in the browser! Bundle sizes of this minimal app have gone from 100MB raw, 45 compressed -> 50MB raw, 30 compressed.

@Thunkar Thunkar added the e2e-prover-full CI: Enables this CI job. label Dec 19, 2024
@Thunkar Thunkar self-assigned this Dec 19, 2024
@Thunkar Thunkar added e2e-all CI: Enables this CI job. and removed e2e-prover-full CI: Enables this CI job. labels Dec 19, 2024
…of github.com:AztecProtocol/aztec-packages into gj/reduce_bundle_sizes
@Thunkar Thunkar requested a review from spalladino December 19, 2024 14:14
Copy link
Contributor

@benesjan benesjan left a comment

Choose a reason for hiding this comment

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

What a monster diff. LGTM

Copy link
Collaborator

@LeilaWang LeilaWang left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Collaborator

@spalladino spalladino left a comment

Choose a reason for hiding this comment

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

Hell of a PR, thanks for tackling this! IMO it's good to merge once the yarn thingie is addressed. The removal of debug symbols was a good call.

More in general, while this change is great for reducing client bundle size, I'm worried that it's too easy to screw up via in inadvertent import in the wrong place. We could keep it in check by adding an error to the web build of aztec.js so that it fails loudly if it goes back over a certain size.

But I'd like to open the discussion for a larger refactor in the future. Many of these problems arise because we don't have a clear separation between client and server code in our codebase, everything is expected to be isomporhic where it very rarely is. I think we should refactor circuits.js, types, circuit-types, foundation, etc into a common/shared, common/server, and common/client (maybe this last one ends up empty?) set of packages, where we make it very clear where we expect code to run. Granted, this would be a stupidly large change, and would need buy-in from the rest of engineering; but if you find yourself going down the granular imports rabbit hole again, I say we consider it more seriously.

barretenberg/acir_tests/sol-test/package.json Outdated Show resolved Hide resolved
noir-projects/noir-protocol-circuits/package.json Outdated Show resolved Hide resolved
boxes/yarn.lock Outdated Show resolved Hide resolved
@Thunkar Thunkar merged commit bbee6c6 into master Dec 20, 2024
77 checks passed
@Thunkar Thunkar deleted the gj/reduce_bundle_sizes branch December 20, 2024 13:43
TomAFrench added a commit that referenced this pull request Dec 20, 2024
* master: (119 commits)
  chore(master): Release 0.68.0 (#10834)
  feat: enable profiling with local PXE in cli-wallet (#10736)
  chore: values for sepolia deployment (#10362)
  fix: vm_full_tests.yml (#10912)
  chore: disable bb-sanitizers.yml (#10901)
  chore: Check yarn version during bootstrap (#10910)
  fix(p2p): default peer score penalties (#10896)
  feat: Updated metrics (#10885)
  feat: Reduce bundle sizes, remove polyfills (#10877)
  chore: Replace bbup.dev link (#10908)
  chore(avm): extra column information in lookups (#10905)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  chore: disable flake in e2e_fees/private_payments (#10898)
  chore(ci): disable e2e_cheat_codes.test.ts (#10897)
  fix: increase default heartbeat (#10891)
  fix(p2p): less verbose error (#10886)
  refactor: `contact` --> `sender` in PXE API (#10861)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e-all CI: Enables this CI job.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants