Skip to content

Faster sync by collecting esplora ureq thread handles #579

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

Merged
merged 1 commit into from
Apr 6, 2022

Conversation

nickfarrow
Copy link
Contributor

Description

Speeds up esplora ureq syncing. Taken from #560

The current sync just creates a map of scripts to joinhandles which doesn't yet spawn the sync threads due to lazy evaluation. In the following handles.map(), the thread handles are sequentially evaluated and joined. With the fix, the handles are collected so that the threads spawn in parallel, and then joined

Notes to the reviewers

I had to add a #[allow(clippy::needless_collect)] so that it wouldn't complain about collecting and then iterating. (Perhaps clippy is partially responsible for this issue!)

Tested sync performance by doing a fresh sync on an existing gun wallet.

---- Before fix ---
real	0m13.121s
real	0m13.367s
real	0m13.211s

---- After fix ----
real	0m5.516s
real	0m5.251s
real	0m5.594s

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@notmandatory
Copy link
Member

Now that you're fixing the parallel execution should we worry about running out of some resource if a wallet has lots of scripts and transactions to sync? seems like this could swamp a low power esplora server or at least exceed some point of diminishing returns for spawning new threads. You don't need to address it in this PR but but do you have any thoughts for how to address this issue? maybe some sort of thread pool?

@LLFourn
Copy link
Contributor

LLFourn commented Apr 5, 2022

Now that you're fixing the parallel execution should we worry about running out of some resource if a wallet has lots of scripts and transactions to sync? seems like this could swamp a low power esplora server or at least exceed some point of diminishing returns for spawning new threads. You don't need to address it in this PR but but do you have any thoughts for how to address this issue? maybe some sort of thread pool?

Note that the number of requests is limited. It does them in batches according to concurrency.

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK adef166

@notmandatory notmandatory merged commit 0621ca8 into bitcoindevkit:master Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants