Skip to content

Commit

Permalink
Avoid iterator_nextv() call if end was reached
Browse files Browse the repository at this point in the history
  • Loading branch information
vweevers committed Feb 4, 2024
1 parent e7e2763 commit 6b7b282
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 6 deletions.
27 changes: 23 additions & 4 deletions binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ static std::map<std::string, LevelDbHandle> db_handles;
} \
}

/**
* Bit fields.
*/
#define STATE_ENDED 1

/*********************************************************************
* Helpers.
********************************************************************/
Expand Down Expand Up @@ -926,7 +931,8 @@ struct Iterator final : public BaseIterator {
const bool fillCache,
const Encoding keyEncoding,
const Encoding valueEncoding,
const uint32_t highWaterMarkBytes)
const uint32_t highWaterMarkBytes,
unsigned char* state)
: BaseIterator(database, reverse, lt, lte, gt, gte, limit, fillCache),
id_(id),
keys_(keys),
Expand All @@ -939,6 +945,7 @@ struct Iterator final : public BaseIterator {
isClosing_(false),
aborted_(false),
ended_(false),
state_(state),
ref_(NULL) {
}

Expand Down Expand Up @@ -1001,6 +1008,7 @@ struct Iterator final : public BaseIterator {
bool isClosing_;
bool aborted_;
bool ended_;
unsigned char* state_;
std::vector<Entry> cache_;

private:
Expand Down Expand Up @@ -1707,10 +1715,15 @@ static void FinalizeIterator (napi_env env, void* data, void* hint) {
* Create an iterator.
*/
NAPI_METHOD(iterator_init) {
NAPI_ARGV(2);
NAPI_ARGV(3);
NAPI_DB_CONTEXT();

napi_value options = argv[1];
unsigned char* state = 0;
size_t stateLength;
NAPI_STATUS_THROWS(napi_get_typedarray_info(env, argv[1], NULL, &stateLength, (void**)&state, NULL, NULL));
assert(stateLength == 1);

napi_value options = argv[2];
const bool reverse = BooleanProperty(env, options, "reverse", false);
const bool keys = BooleanProperty(env, options, "keys", true);
const bool values = BooleanProperty(env, options, "values", true);
Expand All @@ -1728,7 +1741,8 @@ NAPI_METHOD(iterator_init) {
const uint32_t id = database->currentIteratorId_++;
Iterator* iterator = new Iterator(database, id, reverse, keys,
values, limit, lt, lte, gt, gte, fillCache,
keyEncoding, valueEncoding, highWaterMarkBytes);
keyEncoding, valueEncoding, highWaterMarkBytes,
state);
napi_value result;

NAPI_STATUS_THROWS(napi_create_external(env, iterator,
Expand Down Expand Up @@ -1862,6 +1876,11 @@ struct NextWorker final : public BaseWorker {
napi_set_element(env, jsArray, idx, element);
}

// TODO: use state_ internally too, replacing ended_?
if (iterator_->ended_) {
*iterator_->state_ |= STATE_ENDED;
}

napi_resolve_deferred(env, deferred, jsArray);
}

Expand Down
20 changes: 18 additions & 2 deletions iterator.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@ const kContext = Symbol('context')
const kCache = Symbol('cache')
const kFirst = Symbol('first')
const kPosition = Symbol('position')
const kState = Symbol('state')
const kSignal = Symbol('signal')
const kAbort = Symbol('abort')
const empty = []

// Bit fields
const STATE_ENDED = 1

// Does not implement _all() because the default implementation
// of abstract-level falls back to nextv(1000) and using all()
// on more entries than that probably isn't a realistic use case,
Expand All @@ -20,7 +24,8 @@ class Iterator extends AbstractIterator {
constructor (db, context, options) {
super(db, options)

this[kContext] = binding.iterator_init(context, options)
this[kState] = new Uint8Array(1)
this[kContext] = binding.iterator_init(context, this[kState], options)
this[kFirst] = true
this[kCache] = empty
this[kPosition] = 0
Expand All @@ -38,17 +43,22 @@ class Iterator extends AbstractIterator {
_seek (target, options) {
this[kFirst] = true
this[kCache] = empty
this[kState][0] &= ~STATE_ENDED // Unset
this[kPosition] = 0

binding.iterator_seek(this[kContext], target)
}

// TODO (v2): document/benchmark that we now always need a last call
async _next () {
if (this[kPosition] < this[kCache].length) {
return this[kCache][this[kPosition]++]
}

// Avoid iterator_nextv() call if end was already reached
if ((this[kState][0] & STATE_ENDED) !== 0) {
return undefined
}

if (this[kFirst]) {
// It's common to only want one entry initially or after a seek()
this[kFirst] = false
Expand All @@ -69,6 +79,12 @@ class Iterator extends AbstractIterator {
// TODO (v2): read from cache first?
async _nextv (size, options) {
this[kFirst] = false

// Avoid iterator_nextv() call if end was already reached
if ((this[kState][0] & STATE_ENDED) !== 0) {
return []
}

return binding.iterator_nextv(this[kContext], size)
}

Expand Down

0 comments on commit 6b7b282

Please sign in to comment.