implement parallel processing in sync() method #843
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
* sdk: fix tests not building * sdk: tweak polling tx confirmation
* sdk: add option to get signed settlepnl tix from a market order * fix lint
* bigz/add-lp-order-risk-mitigations * friendly perp position iter * update parmams mutability * add time_since_last_liquidity_change * add new variable to margin context mode * add attempt to burn shares in force cancel/settle pnl * remove from force cancel orders * sdk: fix tests not building * change place orders * rmeove unused mut * address some comments * add to settle_pnl * use oracle orders * tweak order params * oracle order tests * add typescript test, fix order param market_type * remove logic from settle_expired_position * CHANGELOG --------- Co-authored-by: Chris Heaney <[email protected]>
This reverts commit 21c4a46.
This reverts commit 63c7208.
* Added handling for signature time measurement * Added opt-in metrics flag to drift client * fixed prettier --------- Co-authored-by: lowkeynicc <[email protected]>
sdk: fix ts timeout
no state ws resub
…840) * bigz/add-oracle-volatility-protections * fix pct vs price bug, add test + changelog * cleanup code (naming, mutables)
* program: init pause_operations flag * tweaks * write tests for paused operations * split out spot and perp operations * fixes * add sdk and use PerpOperation/SpotOperation naming convention * fix update funding ix * stop update to deprecated market status * add admin fn * add logging for admin fn * CHANGELOG
| this.updateLatestSlot(slot); | ||
|
|
||
| const programAccountBufferMap = new Map<string, Buffer>(); | ||
| for (const programAccount of rpcResponseAndContext.value) { | ||
| rpcResponseAndContext.value.forEach(programAccount => { |
There was a problem hiding this comment.
Have you benchmarked this change against whatever the average response length received is?
Last I checked forEach was the least performant way to iterate in Javascript.
There was a problem hiding this comment.
i dont think it makes much difference with for...of, main improvement in term of execution time i have benchmarked when calling userMap.subscribe using keeper bot went from 120 seconds to 11 on my local machine was due to parallel processing, which is right after this iteration. i apologize in advance im still learing and improving
wphan
reviewed
Feb 27, 2024
| const promises = Array.from(programAccountBufferMap.entries()).map(async ([key, buffer]) => { | ||
| const index = await Promise.race(semaphore.map((p, index) => p.then(() => index))); | ||
| semaphore[index] = processAccount(key, buffer).then(() => { | ||
| return; |
There was a problem hiding this comment.
you shouldn't need this .then..return
| }); | ||
|
|
||
| const concurrencyLimit = 20; | ||
| const semaphore = new Array(concurrencyLimit).fill(Promise.resolve()); |
There was a problem hiding this comment.
what's the reason for this semaphore? does it still work if you just let all the promises run at once?
wphan
reviewed
Feb 27, 2024
Comment on lines
+335
to
+342
| const promises = Array.from(programAccountBufferMap.entries()).map(async ([key, buffer]) => { | ||
| const index = await Promise.race(semaphore.map((p, index) => p.then(() => index))); | ||
| semaphore[index] = processAccount(key, buffer).then(() => { | ||
| return; | ||
| }); | ||
| }); | ||
|
|
||
| await Promise.all(promises.concat(semaphore)); |
There was a problem hiding this comment.
Suggested change
| const promises = Array.from(programAccountBufferMap.entries()).map(async ([key, buffer]) => { | |
| const index = await Promise.race(semaphore.map((p, index) => p.then(() => index))); | |
| semaphore[index] = processAccount(key, buffer).then(() => { | |
| return; | |
| }); | |
| }); | |
| await Promise.all(promises.concat(semaphore)); | |
| const promises = Array<Promise<void>>(); | |
| programAccountBufferMap.forEach(async (buffer, key) => { | |
| promises.push(processAccount(key, buffer)); | |
| }); | |
| await Promise.all(promises); |
you should be able to reduce it to this, not sure if semaphore is necssary
|
curious to try this out more throughly, gonna change the target branch and finish the merge there. Thank you @didobagi ! |
wphan
force-pushed
the
merge/didobagi/userMapSync
branch
from
721ab0f to
8698a43
Compare
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.
No description provided.