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

Get account data improvement #243

Merged
merged 13 commits into from
Mar 4, 2018
Merged

Conversation

Cylix
Copy link
Collaborator

@Cylix Cylix commented Feb 26, 2018

Small improvement of getAccountData.

  • Remove the returnAll parameter which is kind of pointless for that function
  • Reuse the return of getNewAddresses for transfers & balances (avoid regenerating the same addresses over and over again).

The improvement results in twice as fast on testing data (as demonstrated by the screenshot below).
The improvement should be more significant for accounts with a large number of addresses.

Before:
screen shot 2018-02-25 at 6 55 09 pm

After:
screen shot 2018-02-25 at 7 05 45 pm

@Cylix
Copy link
Collaborator Author

Cylix commented Feb 26, 2018

I also addressed #194, but turns out it does not improve much (tried different seeds).
I case the case covered by the improvement does not happen frequently enough.

This was referenced Feb 26, 2018
@Cylix
Copy link
Collaborator Author

Cylix commented Feb 27, 2018

Please do not merge immediately, still doing some experiments.

@Cylix
Copy link
Collaborator Author

Cylix commented Mar 1, 2018

Just addressed #194 optimization comment. It now goes as fast as Java library and can go a little bit faster by doubling the number of threads in the parallel_for.

Before merging this issue, I will check some places where we can initialize the hash in Models::Bundle (I added Models::Bundle::getHash and Models::Bundle::setHash to improve API & code clarity).

@thibault-martinez thibault-martinez merged commit e49c2ad into master Mar 4, 2018
@thibault-martinez thibault-martinez deleted the get-account-data-improvement branch March 28, 2018 10:41
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.

None yet

2 participants