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

Rewrite project as thin wrapper on react-native-leveldb #7

Open
andymatuschak opened this issue Mar 3, 2021 · 3 comments
Open

Rewrite project as thin wrapper on react-native-leveldb #7

andymatuschak opened this issue Mar 3, 2021 · 3 comments

Comments

@andymatuschak
Copy link
Owner

andymatuschak commented Mar 3, 2021

Now that @savv has written a TurboModules implementation of the native LevelDB bindings (https://github.com/greentriangle/react-native-leveldb), this library should become a thin abstract-leveldown adapter for that interface:

  • that native module will be much faster than this one, since TurboModules has a much more efficient FFI (avoids marshaling and encoding/decoding data)
  • that library supports null-terminated keys/values (see strings are incorrectly null-terminated #6)
  • that library has a shared iOS / Android implementation, lowering the maintenance burden

One observation: that library supports synchronous access of the DB, while abstract-leveldown mandates asynchronous semantics (i.e. inserting nextTick even when data can be requested immediately). That seems like a shame; perhaps we should have some special high-performance option which retains the synchronous semantics, at the cost of breaking abstract-leveldown's assumptions.

@andymatuschak
Copy link
Owner Author

I don't expect I'll get to this anytime soon; patches welcome! Should be pretty straightforward…

@savv
Copy link

savv commented Mar 4, 2021

Hi Andy, nice to see this! As I've said before, let me know if you run into any kinks (e.g. I just discovered that I forgot Prev(), planning to add it soon.)

I wanted to add to one thing you said -

One observation: that library supports synchronous access of the DB, while abstract-leveldown mandates asynchronous semantics (i.e. inserting nextTick even when data can be requested immediately). That seems like a shame; perhaps we should have some special high-performance option which retains the synchronous semantics, at the cost of breaking abstract-leveldown's assumptions.
This sounds like a great idea! I see the trade-off as follows:

For many types of usages (e.g. ~100 reads/writes in response to a user action) I bet that keeping it sync is a good trade-off. Maybe we could bake something like this in your wrapper, where it yields execution every XX operations, or even make it an option.

If somebody is used to the async nature of leveldown, they might be surprised to see their app locking, if they fetch a larger number of keys (thousands+); previously, the async implementations of leveldown would have yielded execution to other parts of the app (like rendering views).

@andymatuschak
Copy link
Owner Author

Yep, I agree with your broad conclusions: keeping it async as default probably promotes "least surprise," especially when coupled with reasonable heuristics like batching iterator nexts (which this library currently does). More synchronous behavior should probably be opt-in.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants