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

Optimize db.clear() and db.iterator() #784

Merged
merged 5 commits into from
Sep 24, 2021
Merged

Optimize db.clear() and db.iterator() #784

merged 5 commits into from
Sep 24, 2021

Conversation

vweevers
Copy link
Member

@vweevers vweevers commented Sep 12, 2021

Closes #680. To be (partially) squashed before merge; I separated this into small refactoring commits for easier review.

By doing db.clear() natively, without crossing the C++/JS boundary for every key, it's 27x faster. That's compared to master, shown as "baseline" in the graphs.

clear 1632180672020

After further refactoring and tweaking, db.iterator() also became slightly faster. I also fixed an edge case where a db.close() would never call its callback if a iterator().next() was in progress and that next() call errored.

iterate 1632180713566

@vweevers vweevers added the semver-patch Bug fixes that are backward compatible label Sep 12, 2021
binding.cc Outdated Show resolved Hide resolved
@vweevers vweevers changed the title Optimize db.clear() by doing it natively Optimize db.clear() and db.iterator() Sep 20, 2021
@vweevers vweevers marked this pull request as ready for review September 20, 2021 23:47
@vweevers
Copy link
Member Author

Relevant benchmarks are in Level/bench#29.

Copy link
Member

@ralphtheninja ralphtheninja left a comment

Choose a reason for hiding this comment

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

Amazing work!

binding.cc Show resolved Hide resolved
binding.cc Show resolved Hide resolved
Because this uses an iterator under the hood, it also refactors
shared code between `db.iterator()` and `db.clear()`.
By using `emplace_back()`, reusing the `std::vector` cache between
`iterator.next()` calls, and not advancing the iterator prematurely.
That last one only matters for single reads (i.e. the first `next()`
call or one made after seeking) and it doesn't improve performance
compared to the previous release, just undoes a mistake in b815bea.
@vweevers
Copy link
Member Author

vweevers commented Sep 24, 2021

Squashed. The test failure can be ignored:

Run codecov/codecov-action@v2
==> macos OS detected
Error: read ECONNRESET
    at TLSWrap.onStreamRead (internal/stream_base_commons.js:201:27)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch Bug fixes that are backward compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize _clear()
2 participants