Skip to content
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

Fix redundant calls to save_addresses during synchronization #2113

Merged
merged 1 commit into from
Dec 30, 2020

Conversation

mhsmith
Copy link
Collaborator

@mhsmith mhsmith commented Dec 29, 2020

Currently save_addresses is called after every address is added. Since this copies the entire address list to storage every time, the cost is n**2 in the number of addresses. For wallets with thousands of addresses, the initial synchronization process was spending more than half of its time in save_addresses.

So I've changed it to save the addresses in the same situations as it saves the transactions:

  • When synchronization is complete.
  • When the wallet is closed.

This reduced the synchronization time of an 11,000-address wallet from 20 minutes to 6.

@cculianu cculianu added the performance / optimization Issues related to speeding up the performance of the app label Dec 30, 2020
@cculianu
Copy link
Collaborator

This looks great! Nice catch. It should also help with the desktop version since users of that version tend to have tens of thousands of addresses. Let me review this in more detail tomorrow and merge it. Thanks Malcom!

@cculianu
Copy link
Collaborator

Reviewed, looks great. In the very unlikely event that the app crashes or is killed and the addresses don't get saved, it's no big deal either. Presumably the synchronizer will just have to fast-forward again from a previous state. This commit is pure optimization for the common case with zero drawbacks. The fact that it saves addresses on up_to_date is a nice touch.

Merging!

@cculianu cculianu merged commit 56c89bc into Electron-Cash:master Dec 30, 2020
@mhsmith
Copy link
Collaborator Author

mhsmith commented Dec 30, 2020

In the very unlikely event that the app crashes or is killed and the addresses don't get saved, it's no big deal either.

It's a lot more likely on mobile because the user might turn the device off, or the OS might kill the app once it goes to the background. So I'm going to add an auto-save feature to the Android app: I'll write it in Python so you can use it in the iOS app if you want.

@cculianu
Copy link
Collaborator

Yeah and the “penalty” in that case if it happens it more expensive/more painful on mobile. Good point.

Sure please do share and I will definitely use it. Cool stuff.

Alternatively, just a dumb heuristic in synchronizer to “Sometimes” pass save=True there might be enough. Say every 10 or 20 times .. something like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance / optimization Issues related to speeding up the performance of the app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants