DecryptNotes: move the code that groups notes by account from WorkerPool to the response class #5089
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
jowparks
approved these changes
Jun 27, 2024
| accounts: ReadonlyArray<{ accountId: string }>, | ||
| ): Map<string, Array<DecryptedNote | null>> { | ||
| const decryptedNotesByAccount: Array< | ||
| [accountId: string, notes: Array<DecryptedNote | null>] |
There was a problem hiding this comment.
.map is much slower in some cases than for..of. Not sure if this is one of those cases just checking to see if you are aware
There was a problem hiding this comment.
It's true that a for loop can outperform .map(), but in this very specific case I'm trying to get an array out of another array, so I'd expect the performance difference to be minimal. I would be more concerned if I was doing chains of .map/.filter, because those create a new array each time, and that would be better written as a for loop (at least until Node.js gains the ability to optimize these methods). Thanks for pointing it out though.
andiflabs
force-pushed
the
andrea/move-decryptnotes-response-processing-out-of-pool
branch
from
501ed5d to
d7a9699
Compare
…ool to the response class
andiflabs
force-pushed
the
andrea/move-decryptnotes-response-processing-out-of-pool
branch
from
d7a9699 to
5fdde88
Compare
andiflabs
deleted the
andrea/move-decryptnotes-response-processing-out-of-pool
branch
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.
Summary
The goal of this PR is to move that chunk of code to a location where it's more easily reusable. The plan is to use the new method introduced here in PR-5058: that PR will be changed to not call
WorkerPool.decryptNotes()anymore, but rather callWorkerPool.execute()to obtain the individual jobs.Testing Plan
Unit tests
Documentation
N/A
Breaking Change
N/A