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

refactor benchmarks to avoid DeferredPromise #238

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

staltz
Copy link
Member

@staltz staltz commented Jul 12, 2021

Two main changes here:

  • Removed the DeferredPromise in benchmarks/index.js
    • I thought this was the only way of doing it, but I figured out a simpler way, which is just await new Promise((resolve) =>
  • Moved the "add 1000 messages" benchmark to the bottom
    • I noticed that since this was the first test, then it meant that all subsequent tests were running over 101k messages, not 100k messages as ssb-fixtures was configured for. I guess we should stick to 100k messages for the tests because it helps us know how much time "per message" on average is spent. We should not add messages to these tests.

@github-actions
Copy link

Benchmark results

Part Duration
Migrate (+db1) 9146ms
Migrate (alone) 4324ms
Migrate (+db1 +db2) 7737ms
Migrate (+db2) 3760ms
Migrate continuation (+db2) 1311ms
Memory usage without indexes 764.20 MB = 34.33 MB + etc
Initial indexing 1111ms
Initial indexing maxCpu=86 5865ms
Initial indexing compat 945ms
Two indexes updating concurrently 1216ms
key one initial 53ms
key two 3ms
key one again 6ms
reboot and key one again 69ms
latest root posts 593ms
latest posts 46ms
votes one initial 693ms
votes again 1ms
hasRoot 424ms
hasRoot again 1ms
author one posts 371ms
author two posts 30ms
dedicated author one posts 426ms
dedicated author one posts again 1ms
add 1000 elements 184ms
Maximum memory usage 791.80 MB = 43.39 MB + etc
Indexes folder size 12.23mb

@staltz staltz requested a review from arj03 July 12, 2021 16:13
Copy link
Member

@arj03 arj03 left a comment

Choose a reason for hiding this comment

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

I was looking at the old file and the new and this does seem to improve readability overall. This will probably mean I need to rebase some PRs but that is okay, I want to rebase them on top of validate2 PR anyway.

@staltz
Copy link
Member Author

staltz commented Jul 12, 2021

@arj03 if you want, I can do this refactor after the big merges happen.

@arj03
Copy link
Member

arj03 commented Jul 12, 2021

Might be a good way to review it :)

@staltz staltz marked this pull request as draft July 13, 2021 08:15
@staltz
Copy link
Member Author

staltz commented Jul 13, 2021

I'll park this PR for now, I can copy-paste or reuse parts of it later.

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.

2 participants