feat: Faster random sampling #9655
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rumata888
force-pushed
the
is/even_faster_random
branch
from
2b6412f to
20c0bd5
Compare
Rumata888
force-pushed
the
is/even_faster_random
branch
from
20c0bd5 to
417c055
Compare
Rumata888
commented
Nov 1, 2024
| hi.self_reduce_once(); | ||
| hi.self_reduce_once(); | ||
| hi.self_reduce_once(); | ||
| lo = engine->get_random_uint256(); |
There was a problem hiding this comment.
I restored the version that Zac checked in a previous PR. Accidentally got in an unchecked PR in there
ludamad
reviewed
Nov 1, 2024
| #ifndef NO_MULTITHREADING | ||
| std::unique_lock<std::mutex> lock(random_buffer_mutex); | ||
| #endif | ||
|
|
There was a problem hiding this comment.
This seems like it'd overall work better if the random buffer and random buffer offset were thread_local, then we wouldn't need any locking?
There was a problem hiding this comment.
Yes, I'll switch. Hope we don't get a situation where we sample very few elements in new threads all the time
There was a problem hiding this comment.
with the thread pool, that wouldn't be the case
|
Still have a style complaint that we added two related global variables not bundled in a struct, but approving |
ludamad
approved these changes
Nov 1, 2024
TomAFrench
added a commit
that referenced
this pull request
Nov 4, 2024
* master: (81 commits) feat: Encode static error strings in the ABI (#9552) chore: redo typo PR by donatik27 (#9693) chore: update install instructions for foundry to display version of rust to install (#9653) chore: disable bench upload until #9692 fix: earthly-ci in bench-e2e (#9689) chore: redo typo PR by cypherpepe (#9687) chore: redo typo PR by youyyytrok (#9686) chore: redo typo PR by mdqst (#9685) chore: redo typo PR by mdqst (#9684) feat: adding tags to encrypted logs (#9566) fix: enable gerousia e2e test (#9677) 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: redo typo PR by dsarfed (#9667) fix: bench e2e jobs (#9662) fix: Fix random for Mac users (#9670) feat: Graph methods for circuit analysis (part 1) (#7948) feat: Faster random sampling (#9655) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR:
169x in Native, 35x in wasm
Before, native:
After:
Before, wasm:
After: