Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

Rewrite to N-API #111

Merged
merged 7 commits into from
May 25, 2019
Merged

Rewrite to N-API #111

merged 7 commits into from
May 25, 2019

Conversation

vweevers
Copy link
Member

Closes #100. Though this looks like a big PR, there's not much code to review because I copied large parts from leveldown. That includes binding.cc and all JS files including tests (rocksdb has only one test of its own: open-read-only-test.js). The differences between leveldown and rocksdb are recorded in 3433541, 7a5547b and bcdd317.

Prebuilds

Like leveldown, rocksdb now uses prebuildify which means prebuilds will be included in the npm package. Unlike leveldown, rocksdb does not make prebuilds for ARM, Android or Alpine (until someone requests them, I figured) (and on RPi and mobile you're better off with RocksDBLite). After initial review of this PR I'll trigger CI to make prebuilds, so we can check what the total npm package size will be.

Additional changes

  • Disable block cache if cacheSize is 0, rather than using a block cache with RocksDB's default size
  • Set verify_checksums to false (same as leveldown / LevelDB) (Support checksum options leveldown#172 (comment))
  • Enable bloom filter (same as leveldown / LevelDB)
  • Increase info_log_level from INFO_LEVEL to WARN_LEVEL

Performance

The leak tests (node --expose-gc test/leak-tester[-batch]) show memory growth but I get the same results on master (I used node 10.14.1 on both) so it might be a preexisting leak or natural RocksDB growth.

I want to run benchmarks as well; perhaps by extracting the benchmarks from leveldown into a standalone tool that can target any abstract-leveldown implementation.

@vweevers vweevers added semver-major Changes that break backward compatibility refactor Requires or pertains to refactoring labels May 20, 2019
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.

Nice work. LGTM!

@vweevers

This comment has been minimized.

@ralphtheninja

This comment has been minimized.

@vweevers

This comment has been minimized.

@vweevers
Copy link
Member Author

vweevers commented May 22, 2019

Source code alone, package size is 1.8 MB, 8.5 MB unpacked. With prebuilds, the package size is 5.9 MB, 19.5 MB unpacked.

npm notice 3.5MB   prebuilds/darwin-x64/node.napi.glibc.node
npm notice 3.5MB   prebuilds/linux-x64/node.napi.glibc.node
npm notice 3.9MB   prebuilds/win32-x64/node.napi.glibc.node

(side note: I should remove removed the libc tag from the windows prebuild) (no harm, just looks weird)

@vweevers
Copy link
Member Author

(first test of benchmark code, conditions may not have been optimal)

bench-20190522-22 42 15

@vweevers
Copy link
Member Author

Interesting: this branch of rocksdb is faster (on writes) than master, which is slower than leveldown:

test

I'll try to finish those benchmarks over the weekend so y'all can play too 😄

@vweevers
Copy link
Member Author

vweevers commented May 25, 2019

Benchmark code available here.

With some npm magic you can install rocksdb@3 and the napi master branch side-by-side and compare their performance. Requires npm >= 6.9.0 (for aliases) but also node < 12 (for rocksdb@3). If you can't upgrade npm then use npx npm@latest:

mkdir tmp && cd tmp

npx npm@latest i rocksdb3@npm:rocksdb@3 Level/rocksdb --no-save
npm i level-bench -g

level-bench run write rocksdb3
level-bench run write rocksdb
level-bench plot write

Repeat the last 3 steps for the other benchmarks (write-random and write-sorted). You may need to reduce the amount of data for these benchmarks (gnuplot crashed on me). You can do so with -b [-n 1e6], for example:

level-bench run write-sorted rocksdb3 -b [-n 1e6]
level-bench run write-sorted rocksdb -b [-n 1e6]
level-bench plot write-sorted

Last note: you don't have to npm-install rocksdb, you can also clone the repo, install as usual, and run level-bench run <benchmark> (without further arguments) in that working directory.

Results on Windows (TLDR: we're good, though we don't have read benchmarks yet):

write
write-random
write-sorted

@vweevers
Copy link
Member Author

More benchmarks are welcome, post results here! In the mean time I'm merging this.

@vweevers vweevers merged commit c7f8481 into master May 25, 2019
@vweevers vweevers deleted the napi branch May 26, 2019 14:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
refactor Requires or pertains to refactoring semver-major Changes that break backward compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite to N-API
2 participants