Implement explicit snapshots #110
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Level/community#118. TLDR:
This was relatively easy to implement because it's merely exposing an existing LevelDB feature. Which we were already using too (for example, creating an iterator internally creates a snapshot; I now refer to that as an "implicit" snapshot). The only challenge was garbage collection and state management. For that I knew I could copy iterator logic but I wanted to avoid its technical debt, so I first cleaned that up:
binding.iterator_close()is now synchronous, because that's 2x faster thannapi_create_async_work, let alone executing that async work. Measured withperformance.timerify(binding.iterator_close). Simplifies state.Database#ref_) from a weak to a strong reference, preventing GC untildb.close(). I thought this would remove the need for strong references to iterators & snapshots seeing as theAbstractLevelclass already has references in thedb[kResources]set, but a new unit test initerator-gc-test.jsproved me wrong. So I kept that as-is (would like to revisit).IncrementPriorityWorkfunction to increment the refcount of said reference. It now just increments astd::atomic<int>(for our own state outside of V8).IncrementPriorityWorkwhich had 2 purposes:abstract-levelwhich closes iterators & snapshots before callingdb._close().I then moved common logic for references and teardown to a new struct called
Resource, inherited byIteratorandExplicitSnapshot. The latter is a new struct that wraps a LevelDB snapshot.Keeping snapshots open during read operations like
db.get()is handled inabstract-levelthroughAbstractSnapshot#ref().I think further cleanup is possible (and to move more state management to JS) but I'll save that for a future PR.
This PR depends on Level/abstract-level#93 but is otherwise complete.