-
-
Notifications
You must be signed in to change notification settings - Fork 446
fix: avoid Buffer.from copies #6723
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
Changes from 9 commits
4323740
ae68206
5d26127
3e5984e
5a71120
dc549de
5fd1a8d
9f9904e
c2832c2
1c687b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -90,6 +90,8 @@ function innerShuffleList(input: Shuffleable, seed: Bytes32, dir: boolean): void | |
| const listSize = input.length >>> 0; | ||
| // check if list size fits in uint32 | ||
| assert.equal(listSize, input.length, "input length does not fit uint32"); | ||
| // check that the seed is 32 bytes | ||
| assert.lte(seed.length, 32, "seed length is not lte 32 bytes"); | ||
wemeetagain marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| const buf = Buffer.alloc(_SHUFFLE_H_TOTAL_SIZE); | ||
| let r = 0; | ||
|
|
@@ -100,8 +102,7 @@ function innerShuffleList(input: Shuffleable, seed: Bytes32, dir: boolean): void | |
| } | ||
|
|
||
| // Seed is always the first 32 bytes of the hash input, we never have to change this part of the buffer. | ||
| const _seed = seed; | ||
| Buffer.from(_seed).copy(buf, 0, 0, _SHUFFLE_H_SEED_SIZE); | ||
| buf.set(seed, 0); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. memory copy might be intentional here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This update is fine, I think more eyes on this change will confirm that.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. confirm it's fine, maybe more clear to bound |
||
|
|
||
| // initial values here are not used: overwritten first within the inner for loop. | ||
| let source = seed; // just setting it to a Bytes32 | ||
|
|
@@ -114,8 +115,8 @@ function innerShuffleList(input: Shuffleable, seed: Bytes32, dir: boolean): void | |
| buf[_SHUFFLE_H_SEED_SIZE] = r; | ||
| // Seed is already in place, now just hash the correct part of the buffer, and take a uint64 from it, | ||
| // and modulo it to get a pivot within range. | ||
| const h = digest(buf.slice(0, _SHUFFLE_H_PIVOT_VIEW_SIZE)); | ||
| const pivot = Number(bytesToBigInt(h.slice(0, 8)) % BigInt(listSize)) >>> 0; | ||
| const h = digest(buf.subarray(0, _SHUFFLE_H_PIVOT_VIEW_SIZE)); | ||
| const pivot = Number(bytesToBigInt(h.subarray(0, 8)) % BigInt(listSize)) >>> 0; | ||
|
|
||
| // Split up the for-loop in two: | ||
| // 1. Handle the part from 0 (incl) to pivot (incl). This is mirrored around (pivot / 2) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.