Skip to content
This repository was archived by the owner on Dec 1, 2024. It is now read-only.
/ rocksdb Public archive

Rewrite to N-API#111

Merged
vweevers merged 7 commits intomasterfrom
napi
May 25, 2019
Merged

Rewrite to N-API#111
vweevers merged 7 commits intomasterfrom
napi

Conversation

@vweevers
Copy link
Copy Markdown
Member

@vweevers vweevers commented May 20, 2019

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
ralphtheninja
ralphtheninja approved these changes May 20, 2019
View reviewed changes
Copy link
Copy Markdown
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!

@NickNaso NickNaso mentioned this pull request May 20, 2019
Closed
@vweevers

This comment has been minimized.

Sign in to view
@ralphtheninja

This comment has been minimized.

Sign in to view
@vweevers

This comment has been minimized.

Sign in to view
@vweevers
Copy link
Copy Markdown
Member Author

vweevers commented May 22, 2019 •
edited
Loading

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
Copy Markdown
Member Author

vweevers commented May 22, 2019

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

bench-20190522-22 42 15

@vweevers
Copy link
Copy Markdown
Member Author

vweevers commented May 24, 2019

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
Copy Markdown
Member Author

vweevers commented May 25, 2019 •
edited
Loading

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
Copy Markdown
Member Author

vweevers commented May 25, 2019

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.

Reviewers

@peakji peakji Awaiting requested review from peakji

@filoozom filoozom Awaiting requested review from filoozom

1 more reviewer

@ralphtheninja ralphtheninja ralphtheninja approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

refactor Requires or pertains to refactoring semver-major Changes that break backward compatibility

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Rewrite to N-API

2 participants

@vweevers @ralphtheninja