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

feat: add option to set max memory for bb.js #4227

Merged
merged 5 commits into from
Mar 4, 2024
Merged

feat: add option to set max memory for bb.js #4227

merged 5 commits into from
Mar 4, 2024

Conversation

signorecello
Copy link
Contributor

@signorecello signorecello commented Feb 1, 2024 •
edited
Loading

Description

Makes BackendBarretenberg pass all the options to the bb.js instance.

Summary*

Makes BackendBarretenberg pass all the options to the bb.js instance, namely min and max memory. Manually setting max memory makes iOS not immediately kill workers on the browser.

This is already implemented in BB as per this PR

Replicates the change I made in a pinch for progrcrypto Istanbul. Example usage here

@signorecello signorecello marked this pull request as ready for review February 2, 2024 10:22
@signorecello
Copy link
Contributor Author

This doesn't break any tests but wouldn't hurt have a NoirJS test for it

@TomAFrench
Copy link
Member

Is there a reason to allow the user to specify the initial memory to be allocated to the backend? I can understand a maximum but in other places where we initialise wasm binaries we generally provide the minimum necessary (and allow it to grow from there).

@signorecello
Copy link
Contributor Author

Good point. I didn't think of that, at the time I simply made optional what was already a parameter in BB.

Seems like below the minimum, we would have all sorts of problems.

I can make the change but that would need another PR in aztec-packages to keep the interface consistent

@TomAFrench
Copy link
Member

Yep, my biggest worry is users passing in an invalid initial memory and then raising issues asking why the wasm doesn't start.

@signorecello
Copy link
Contributor Author

I'll close this one and replicate it in Aztec-Packages, so changes go in the same repo

signorecello added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 13, 2024
@signorecello
replicating changes in noir-lang/noir#4227
30f7f64
@signorecello signorecello mentioned this pull request Feb 13, 2024
Closed
@signorecello
Copy link
Contributor Author

Some discussion later in AztecProtocol/aztec-packages#4570, I'll reopen this as we're keeping the low-level min memory control in BB while not exposing it here

@signorecello signorecello reopened this Feb 14, 2024
@socket-security Socket Security
Copy link

socket-security bot commented Mar 2, 2024 •
edited
Loading

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@cspell/cspell-bundled-dicts@8.4.1 None 0 11.3 kB jason-dent
npm/@cspell/cspell-resolver@8.4.1 None 0 3.77 kB jason-dent
npm/@cspell/cspell-service-bus@8.4.1 None 0 22.2 kB jason-dent
npm/@cspell/cspell-types@8.4.1 None 0 195 kB jason-dent
npm/@cspell/dynamic-import@8.4.1 None +1 85.1 kB jason-dent
npm/@cspell/strong-weak-map@8.4.1 None 0 10.6 kB jason-dent
npm/@types/send@0.17.4 None 0 9.85 kB types
npm/chalk-template@1.1.0 None 0 13.7 kB sindresorhus
npm/clear-module@4.1.2 None 0 6.06 kB sindresorhus
npm/commander@12.0.0 environment, filesystem, shell 0 181 kB abetomo
npm/comment-json@4.2.3 None 0 46.3 kB kael
npm/configstore@6.0.0 None 0 7.58 kB sindresorhus
npm/cspell-gitignore@8.4.1 None +3 118 kB jason-dent
npm/cspell-glob@8.4.1 None +1 84.3 kB jason-dent
npm/cspell-io@8.4.1 None +1 130 kB jason-dent
npm/cspell-lib@8.4.1 None +15 1.25 MB jason-dent
npm/fast-glob@3.3.2 filesystem +1 153 kB mrmlnc
npm/fast-json-stable-stringify@2.1.0 None 0 17 kB esp
npm/file-entry-cache@8.0.0 filesystem 0 16 kB jaredwray
npm/find-up-simple@1.0.0 None 0 7.18 kB sindresorhus
npm/get-stdin@9.0.0 None 0 4.54 kB sindresorhus
npm/import-meta-resolve@4.0.0 None 0 76.9 kB wooorm
npm/micromatch@4.0.5 None 0 55.9 kB jonschlinkert
npm/normalize-url@6.1.0 None 0 21.2 kB sindresorhus
npm/postcss-calc@8.2.4 None 0 188 kB ludovicofischer
npm/source-map-js@1.0.2 None 0 148 kB 7rulnik
npm/strip-ansi@7.1.0 None 0 4.32 kB sindresorhus
npm/vscode-uri@3.0.8 None 0 204 kB vscode-bot

🚮 Removed packages: npm/@babel/helper-module-imports@7.22.15, npm/@babel/helper-string-parser@7.23.4, npm/@babel/helper-validator-identifier@7.22.20, npm/@babel/runtime@7.24.0, npm/@babel/types@7.24.0, npm/@types/parse-json@4.0.2, npm/callsites@3.1.0, npm/convert-source-map@1.9.0, npm/csstype@3.1.3, npm/error-ex@1.3.2, npm/function-bind@1.1.2, npm/hasown@2.0.1, npm/is-arrayish@0.2.1, npm/is-core-module@2.13.1, npm/js-tokens@4.0.0, npm/json-parse-even-better-errors@2.3.1, npm/lines-and-columns@1.2.4, npm/loose-envify@1.4.0, npm/parse-json@5.2.0, npm/path-parse@1.0.7, npm/path-type@4.0.0, npm/react-fast-compare@3.2.2, npm/regenerator-runtime@0.14.1, npm/resolve@1.22.8, npm/source-map@0.5.7, npm/supports-preserve-symlinks-flag@1.0.0, npm/to-fast-properties@2.0.0

View full report↗︎

@TomAFrench TomAFrench added this pull request to the merge queue Mar 4, 2024
@TomAFrench TomAFrench changed the title feat: adding option to set initial and max memory feat: adding option to set max memory for bb.js Mar 4, 2024
@TomAFrench TomAFrench changed the title feat: adding option to set max memory for bb.js feat: add option to set max memory for bb.js Mar 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 4, 2024
@TomAFrench TomAFrench added this pull request to the merge queue Mar 4, 2024
Merged via the queue into master with commit 8a6b131 Mar 4, 2024
47 checks passed
@TomAFrench TomAFrench deleted the zpedro/browser_mem branch March 4, 2024 10:59
@kevaundray kevaundray mentioned this pull request Mar 4, 2024
Merged
TomAFrench added a commit that referenced this pull request Mar 4, 2024
@TomAFrench
Merge branch 'master' into aztec-packages
f82dfc8 
* master:
  chore: export `report_errors` from `nargo` (#4461)
  feat: add option to set max memory for bb.js (#4227)
  feat: backpropagate constants in ACIR during optimization (#3926)
  feat: skip redundant range checks in brillig (#4460)
  fix: Variables from trait constraints being permanently bound over when used within a trait impl (#4450)
  feat: Add overflow and underflow checks for unsigned integers in brillig (#4445)
  fix(flake): stop flake.nix removing ignored-tests.txt (#4455)
  fix: build noir_codegen when publishing (#4448)
  chore: only ignore Nargo.toml in `test_programs` directory and not su… (#4451)
  chore: create parser specifically for function visibility (#4425)
  chore: Document BoundedVec (#4430)
  chore: split up parser into separate files for code organisation (#4420)
  feat: Sync from aztec-packages (#4444)
  chore!: reserve `unchecked` keyword (#4432)
  chore: address code review comments of PR4398 (#4435)
  feat: Sync from aztec-packages (#4439)
  feat: Sync from aztec-packages (#4438)
github-merge-queue bot pushed a commit that referenced this pull request Mar 11, 2024
@kevaundray @Savio-Sou
chore: Release Noir(0.25.0) (#4352)
3f67605 
🤖 I have created a release *beep* *boop*
---


<details><summary>0.25.0</summary>

## [0.25.0](v0.24.0...v0.25.0)
(2024-03-11)


### ⚠ BREAKING CHANGES

* Internal as a macro
(AztecProtocol/aztec-packages#4898)
* reserve `unchecked` keyword
([#4432](#4432))
* Remove empty value from bounded vec
([#4431](#4431))
* Ban Fields in for loop indices and bitwise ops
([#4376](#4376))
* bump msrv to 1.73.0
([#4406](#4406))
* **ci:** Bump MSRV to 1.72.1 and enforce that ACVM can be published
using updated lockfile
([#4385](#4385))
* Restrict bit sizes
([#4235](#4235))
* move noir out of yarn-project
(AztecProtocol/aztec-packages#4479)
* note type ids
(AztecProtocol/aztec-packages#4500)

### Features

* Add eddsa_poseidon_to_pub function to stdlib with test + docs
([#4473](#4473))
([00d2c32](00d2c32))
* Add HashMap to the stdlib
([#4242](#4242))
([650ffc5](650ffc5))
* Add option to set max memory for bb.js
([#4227](#4227))
([8a6b131](8a6b131))
* Add overflow and underflow checks for unsigned integers in brillig
([#4445](#4445))
([21fc4b8](21fc4b8))
* Add poseidon2 opcode implementation for acvm/brillig, and Noir
([#4398](#4398))
([10e8292](10e8292))
* Added cast opcode and cast calldata
(AztecProtocol/aztec-packages#4423)
([78ef013](78ef013))
* Allow type aliases to reference other aliases
([#4353](#4353))
([c44ef14](c44ef14))
* Backpropagate constants in ACIR during optimization
([#3926](#3926))
([aad0da0](aad0da0))
* **ci:** Use wasm-opt when compiling wasm packages
([#4334](#4334))
([e382921](e382921))
* DAP Preflight and debugger compilation options
([#4185](#4185))
([e0ad0b2](e0ad0b2))
* Expose separate functions to compile programs vs contracts in
`noir_wasm` ([#4413](#4413))
([7cd5fdb](7cd5fdb))
* Internal as a macro
(AztecProtocol/aztec-packages#4898)
([5f57ebb](5f57ebb))
* Note type ids
(AztecProtocol/aztec-packages#4500)
([78ef013](78ef013))
* Restrict bit sizes
([#4235](#4235))
([1048f81](1048f81))
* Run tests in parallel in `nargo test`
([#4484](#4484))
([761734e](761734e))
* Skip redundant range checks in brillig
([#4460](#4460))
([cb4c1c5](cb4c1c5))
* Sync from aztec-packages
([#4483](#4483))
([fe8f277](fe8f277))
* Track stack frames and their variables in the debugger
([#4188](#4188))
([ae1a9d9](ae1a9d9))
* TypeVariableKind for just Integers
([#4118](#4118))
([c956be8](c956be8))
* Update error message when trying to load workspace as dependency
([#4393](#4393))
([d2585e7](d2585e7))


### Bug Fixes

* **acir:** Array dynamic flatten
([#4351](#4351))
([b2aaeab](b2aaeab))
* **acir:** Use types on dynamic arrays
([#4364](#4364))
([ba2c541](ba2c541))
* Add `follow_bindings` to follow `Type::Alias` links
([#4521](#4521))
([b94adb9](b94adb9))
* Add handling to `noir_wasm` for projects without dependencies
([#4344](#4344))
([4982251](4982251))
* Allow type aliases in main
([#4505](#4505))
([8a5359c](8a5359c))
* Ban Fields in for loop indices and bitwise ops
([#4376](#4376))
([601fd9a](601fd9a))
* Brillig range check with consistent bit size
([#4357](#4357))
([ea47d4a](ea47d4a))
* Build noir_codegen when publishing
([#4448](#4448))
([cb1ceee](cb1ceee))
* Consistent bit size for truncate
([#4370](#4370))
([dcd7a1e](dcd7a1e))
* Correct formatting for databus visibility types
([#4423](#4423))
([cd796de](cd796de))
* Correct invalid brillig codegen for `EmbeddedCurvePoint.add`
([#4382](#4382))
([5051ec4](5051ec4))
* **docs:** Update install versions
([#4396](#4396))
([b283637](b283637))
* **docs:** Update noirjs_app for 0.23
([#4378](#4378))
([f77f702](f77f702))
* Enforce matching types of binary ops in SSA
([#4391](#4391))
([70866ae](70866ae))
* Fix brillig slowdown when assigning arrays in loops
([#4472](#4472))
([2a53545](2a53545))
* **flake:** Stop flake.nix removing ignored-tests.txt
([#4455](#4455))
([ebaf05a](ebaf05a))
* Force src impl for == on slices
([#4507](#4507))
([1691274](1691274))
* Handling of gh deps in noir_wasm
([#4499](#4499))
([1d65370](1d65370))
* Iterative flattening pass
([#4492](#4492))
([33c1ef7](33c1ef7))
* Noir test incorrect reporting
(AztecProtocol/aztec-packages#4925)
([5f57ebb](5f57ebb))
* Only add `.nr` files to file manager
([#4380](#4380))
([8536c7c](8536c7c))
* Remove panic when generic array length is not resolvable
([#4408](#4408))
([00ab3db](00ab3db))
* Remove print from monomorphization pass
([#4417](#4417))
([27c66b3](27c66b3))
* **ssa:** Handle mergers of slices returned from calls
([#4496](#4496))
([f988d02](f988d02))
* Use correct type for numeric generics
([#4386](#4386))
([0a1d109](0a1d109))
* Variables from trait constraints being permanently bound over when
used within a trait impl
([#4450](#4450))
([ac60ef5](ac60ef5))


### Miscellaneous Chores

* Bump msrv to 1.73.0
([#4406](#4406))
([b5e5c30](b5e5c30))
* **ci:** Bump MSRV to 1.72.1 and enforce that ACVM can be published
using updated lockfile
([#4385](#4385))
([2fc95d2](2fc95d2))
* Move noir out of yarn-project
(AztecProtocol/aztec-packages#4479)
([78ef013](78ef013))
* Remove empty value from bounded vec
([#4431](#4431))
([b9384fb](b9384fb))
* Reserve `unchecked` keyword
([#4432](#4432))
([9544813](9544813))
</details>

<details><summary>0.41.0</summary>

## [0.41.0](v0.40.0...v0.41.0)
(2024-03-11)


### ⚠ BREAKING CHANGES

* Internal as a macro
(AztecProtocol/aztec-packages#4898)
* move noir out of yarn-project
(AztecProtocol/aztec-packages#4479)
* note type ids
(AztecProtocol/aztec-packages#4500)
* rename bigint_neg into bigint_sub
(AztecProtocol/aztec-packages#4420)
* Add expression width into acir
(AztecProtocol/aztec-packages#4014)
* init storage macro
(AztecProtocol/aztec-packages#4200)
* **acir:** Move `is_recursive` flag to be part of the circuit
definition (AztecProtocol/aztec-packages#4221)
* Sync commits from `aztec-packages`
([#4144](#4144))
* Breaking changes from aztec-packages
([#3955](#3955))
* Rename Arithmetic opcode to AssertZero
([#3840](#3840))
* Remove unused methods on ACIR opcodes
([#3841](#3841))
* Remove partial backend feature
([#3805](#3805))

### Features

* Add bit size to const opcode
(AztecProtocol/aztec-packages#4385)
([158c8ce](158c8ce))
* Add expression width into acir
(AztecProtocol/aztec-packages#4014)
([158c8ce](158c8ce))
* Add instrumentation for tracking variables in debugging
([#4122](#4122))
([c58d691](c58d691))
* Add poseidon2 opcode implementation for acvm/brillig, and Noir
([#4398](#4398))
([10e8292](10e8292))
* Add support for overriding expression width
([#4117](#4117))
([c8026d5](c8026d5))
* Added cast opcode and cast calldata
(AztecProtocol/aztec-packages#4423)
([78ef013](78ef013))
* Allow brillig to read arrays directly from memory
(AztecProtocol/aztec-packages#4460)
([158c8ce](158c8ce))
* Allow nested arrays and vectors in Brillig foreign calls
(AztecProtocol/aztec-packages#4478)
([158c8ce](158c8ce))
* Allow variables and stack trace inspection in the debugger
([#4184](#4184))
([bf263fc](bf263fc))
* **avm:** Back in avm context with macro - refactor context
(AztecProtocol/aztec-packages#4438)
([158c8ce](158c8ce))
* **aztec-nr:** Initial work for aztec public vm macro
(AztecProtocol/aztec-packages#4400)
([158c8ce](158c8ce))
* Aztec-packages
([#3754](#3754))
([c043265](c043265))
* Backpropagate constants in ACIR during optimization
([#3926](#3926))
([aad0da0](aad0da0))
* Breaking changes from aztec-packages
([#3955](#3955))
([5be049e](5be049e))
* Evaluation of dynamic assert messages
([#4101](#4101))
([c284e01](c284e01))
* Init storage macro
(AztecProtocol/aztec-packages#4200)
([158c8ce](158c8ce))
* Internal as a macro
(AztecProtocol/aztec-packages#4898)
([5f57ebb](5f57ebb))
* Note type ids
(AztecProtocol/aztec-packages#4500)
([78ef013](78ef013))
* Remove range constraints from witnesses which are constrained to be
constants ([#3928](#3928))
([afe9c7a](afe9c7a))
* Remove replacement of boolean range opcodes with `AssertZero` opcodes
([#4107](#4107))
([dac0e87](dac0e87))
* Speed up transformation of debug messages
([#3815](#3815))
([2a8af1e](2a8af1e))
* Sync `aztec-packages`
([#4011](#4011))
([fee2452](fee2452))
* Sync commits from `aztec-packages`
([#4068](#4068))
([7a8f3a3](7a8f3a3))
* Sync commits from `aztec-packages`
([#4144](#4144))
([0205d3b](0205d3b))
* Sync from aztec-packages
([#4483](#4483))
([fe8f277](fe8f277))


### Bug Fixes

* Deserialize odd length hex literals
([#3747](#3747))
([4000fb2](4000fb2))
* Noir test incorrect reporting
(AztecProtocol/aztec-packages#4925)
([5f57ebb](5f57ebb))
* Remove panic from `init_log_level` in `acvm_js`
([#4195](#4195))
([2e26530](2e26530))
* Return error rather instead of panicking on invalid circuit
([#3976](#3976))
([67201bf](67201bf))


### Miscellaneous Chores

* **acir:** Move `is_recursive` flag to be part of the circuit
definition (AztecProtocol/aztec-packages#4221)
([158c8ce](158c8ce))
* Move noir out of yarn-project
(AztecProtocol/aztec-packages#4479)
([78ef013](78ef013))
* Remove partial backend feature
([#3805](#3805))
([0383100](0383100))
* Remove unused methods on ACIR opcodes
([#3841](#3841))
([9e5d0e8](9e5d0e8))
* Rename Arithmetic opcode to AssertZero
([#3840](#3840))
([836f171](836f171))
* Rename bigint_neg into bigint_sub
(AztecProtocol/aztec-packages#4420)
([158c8ce](158c8ce))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: Savio <72797635+Savio-Sou@users.noreply.github.com>
Savio-Sou
Savio-Sou reviewed Mar 11, 2024
View reviewed changes
tooling/noir_js_backend_barretenberg/src/types.ts
@@ -5,4 +5,5 @@
export type BackendOptions = {
/** @description Number of threads */
threads: number;
memory?: { maximum: number };
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we add a /** @description ... */ to supplement the BackendOptions doc page?

Describing e.g. this being the maximum memory limit, unit of the number (MB? GB?), etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Savio-Sou Savio-Sou Savio-Sou left review comments

@TomAFrench TomAFrench TomAFrench approved these changes

@sirasistant sirasistant Awaiting requested review from sirasistant

@kevaundray kevaundray Awaiting requested review from kevaundray

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@signorecello @TomAFrench @Savio-Sou