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

chore: improve bb.js acir browser tests #8861

Closed
wants to merge 13 commits into from
Closed

chore: improve bb.js acir browser tests #8861

wants to merge 13 commits into from

Conversation

ludamad
Copy link
Collaborator

@ludamad ludamad commented Sep 27, 2024

  • don't create 128 threads
  • don't do that twice

@ludamad ludamad enabled auto-merge (squash) September 27, 2024 19:04
TomAFrench
TomAFrench approved these changes Sep 27, 2024
View reviewed changes
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

Not a fan of the constructor both accepting a preexisting Barretenberg instance and the args to build a new one. Would be nice to have a cleaner distinction but I can handle that in a follow up.

barretenberg/ts/src/barretenberg/verifier.ts Outdated Show resolved Hide resolved
barretenberg/ts/src/barretenberg/backend.ts Outdated Show resolved Hide resolved
@ludamad ludamad changed the title fix: bb.js acir browser tests chore: improve bb.js acir browser tests Sep 27, 2024
@ludamad
Copy link
Collaborator Author

ludamad commented Sep 27, 2024

Thanks yeah appreciate you taking up the improvements

@TomAFrench
Copy link
Member

I've rolled these changes into this PR.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 7, 2024 •
edited
Loading

Changes to circuit sizes

Generated at commit: 92095f311e4758ae40a3aaeab531b9fb593d7d1c, compared to commit: 1b902f663349198aa8f9b3a22663b5c8adc0d442

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
public_kernel_tail -750 ✅ -0.29% +253 ❌ +0.01%
Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
public_kernel_tail 258,422 (-750) -0.29% 2,270,239 (+253) +0.01%

@TomAFrench TomAFrench mentioned this pull request Oct 8, 2024
Merged
TomAFrench added a commit that referenced this pull request Oct 8, 2024
@TomAFrench
fix: limit number of bb.js threads to 32 (#9070)
97e4b9b 
This PR pulls out the limiting of the number of threads from #8861
without the other changes which are failing CI.
@ludamad ludamad closed this Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomAFrench TomAFrench TomAFrench approved these changes

@Rumata888 Rumata888 Awaiting requested review from Rumata888

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

Successfully merging this pull request may close these issues.
2 participants
@ludamad @TomAFrench