From ce3eb77faf6b8c0207d4257fec3bb922ecc1d248 Mon Sep 17 00:00:00 2001 From: Christopher Jeffrey Date: Thu, 7 Apr 2016 19:40:42 -0700 Subject: [PATCH] optimize iterators by avoiding handles. --- src/iterator.cc | 110 +++++++++++++++++++++++------------------------- src/iterator.h | 3 -- src/leveldown.h | 19 +++++++++ 3 files changed, 72 insertions(+), 60 deletions(-) diff --git a/src/iterator.cc b/src/iterator.cc index b941cc5a..cfda47a9 100644 --- a/src/iterator.cc +++ b/src/iterator.cc @@ -31,7 +31,6 @@ Iterator::Iterator ( , bool fillCache , bool keyAsBuffer , bool valueAsBuffer - , v8::Local &startHandle , size_t highWaterMark ) : database(database) , id(id) @@ -51,11 +50,6 @@ Iterator::Iterator ( { Nan::HandleScope scope; - v8::Local obj = Nan::New(); - if (!startHandle.IsEmpty()) - obj->Set(Nan::New("obj").ToLocalChecked(), startHandle); - persistentHandle.Reset(obj); - options = new leveldb::ReadOptions(); options->fill_cache = fillCache; // get a snapshot of the current state @@ -71,11 +65,16 @@ Iterator::Iterator ( Iterator::~Iterator () { delete options; if (start != NULL) { - DisposeStringOrBufferFromSlice(persistentHandle, *start); + // Special case for `start` option: it won't be + // freed up by any of the delete calls below. + if (!((lt != NULL && reverse) + || (lte != NULL && reverse) + || (gt != NULL && !reverse) + || (gte != NULL && !reverse))) { + delete[] start->data(); + } delete start; } - if (!persistentHandle.IsEmpty()) - persistentHandle.Reset(); if (end != NULL) delete end; if (lt != NULL) @@ -341,10 +340,6 @@ v8::Local Iterator::NewInstance ( NAN_METHOD(Iterator::New) { Database* database = Nan::ObjectWrap::Unwrap(info[0]->ToObject()); - //TODO: remove this, it's only here to make LD_STRING_OR_BUFFER_TO_SLICE happy - v8::Local callback; - - v8::Local startHandle; leveldb::Slice* start = NULL; std::string* end = NULL; int limit = -1; @@ -360,6 +355,7 @@ NAN_METHOD(Iterator::New) { v8::Local gtHandle; v8::Local gteHandle; + char *startStr = NULL; std::string* lt = NULL; std::string* lte = NULL; std::string* gt = NULL; @@ -377,12 +373,13 @@ NAN_METHOD(Iterator::New) { && (node::Buffer::HasInstance(optionsObj->Get(Nan::New("start").ToLocalChecked())) || optionsObj->Get(Nan::New("start").ToLocalChecked())->IsString())) { - startHandle = optionsObj->Get(Nan::New("start").ToLocalChecked()).As(); + v8::Local startBuffer = optionsObj->Get(Nan::New("start").ToLocalChecked()); // ignore start if it has size 0 since a Slice can't have length 0 - if (StringOrBufferLength(startHandle) > 0) { - LD_STRING_OR_BUFFER_TO_SLICE(_start, startHandle, start) - start = new leveldb::Slice(_start.data(), _start.size()); + if (StringOrBufferLength(startBuffer) > 0) { + LD_STRING_OR_BUFFER_TO_COPY(_start, startBuffer, start) + start = new leveldb::Slice(_startCh_, _startSz_); + startStr = _startCh_; } } @@ -394,9 +391,9 @@ NAN_METHOD(Iterator::New) { // ignore end if it has size 0 since a Slice can't have length 0 if (StringOrBufferLength(endBuffer) > 0) { - LD_STRING_OR_BUFFER_TO_SLICE(_end, endBuffer, end) - end = new std::string(_end.data(), _end.size()); - DisposeStringOrBufferFromSlice(endBuffer, _end); + LD_STRING_OR_BUFFER_TO_COPY(_end, endBuffer, end) + end = new std::string(_endCh_, _endSz_); + delete _endCh_; } } @@ -418,17 +415,17 @@ NAN_METHOD(Iterator::New) { // ignore end if it has size 0 since a Slice can't have length 0 if (StringOrBufferLength(ltBuffer) > 0) { - LD_STRING_OR_BUFFER_TO_SLICE(_lt, ltBuffer, lt) - lt = new std::string(_lt.data(), _lt.size()); + LD_STRING_OR_BUFFER_TO_COPY(_lt, ltBuffer, lt) + lt = new std::string(_ltCh_, _ltSz_); + delete[] _ltCh_; if (reverse) { - if (start != NULL) { - DisposeStringOrBufferFromSlice(startHandle, *start); - delete start; + if (startStr != NULL) { + delete[] startStr; + startStr = NULL; } - start = new leveldb::Slice(_lt.data(), _lt.size()); - startHandle = optionsObj->Get(Nan::New("lt").ToLocalChecked()).As(); - } else { - DisposeStringOrBufferFromSlice(ltBuffer, _lt); + if (start != NULL) + delete start; + start = new leveldb::Slice(lt->data(), lt->size()); } } } @@ -441,17 +438,17 @@ NAN_METHOD(Iterator::New) { // ignore end if it has size 0 since a Slice can't have length 0 if (StringOrBufferLength(lteBuffer) > 0) { - LD_STRING_OR_BUFFER_TO_SLICE(_lte, lteBuffer, lte) - lte = new std::string(_lte.data(), _lte.size()); + LD_STRING_OR_BUFFER_TO_COPY(_lte, lteBuffer, lte) + lte = new std::string(_lteCh_, _lteSz_); + delete[] _lteCh_; if (reverse) { - if (start != NULL) { - DisposeStringOrBufferFromSlice(startHandle, *start); - delete start; + if (startStr != NULL) { + delete[] startStr; + startStr = NULL; } - start = new leveldb::Slice(_lte.data(), _lte.size()); - startHandle = optionsObj->Get(Nan::New("lte").ToLocalChecked()).As(); - } else { - DisposeStringOrBufferFromSlice(lteBuffer, _lte); + if (start != NULL) + delete start; + start = new leveldb::Slice(lte->data(), lte->size()); } } } @@ -464,17 +461,17 @@ NAN_METHOD(Iterator::New) { // ignore end if it has size 0 since a Slice can't have length 0 if (StringOrBufferLength(gtBuffer) > 0) { - LD_STRING_OR_BUFFER_TO_SLICE(_gt, gtBuffer, gt) - gt = new std::string(_gt.data(), _gt.size()); + LD_STRING_OR_BUFFER_TO_COPY(_gt, gtBuffer, gt) + gt = new std::string(_gtCh_, _gtSz_); + delete _gtCh_; if (!reverse) { - if (start != NULL) { - DisposeStringOrBufferFromSlice(startHandle, *start); - delete start; + if (startStr != NULL) { + delete[] startStr; + startStr = NULL; } - start = new leveldb::Slice(_gt.data(), _gt.size()); - startHandle = optionsObj->Get(Nan::New("gt").ToLocalChecked()).As(); - } else { - DisposeStringOrBufferFromSlice(gtBuffer, _gt); + if (start != NULL) + delete start; + start = new leveldb::Slice(gt->data(), gt->size()); } } } @@ -487,17 +484,17 @@ NAN_METHOD(Iterator::New) { // ignore end if it has size 0 since a Slice can't have length 0 if (StringOrBufferLength(gteBuffer) > 0) { - LD_STRING_OR_BUFFER_TO_SLICE(_gte, gteBuffer, gte) - gte = new std::string(_gte.data(), _gte.size()); + LD_STRING_OR_BUFFER_TO_COPY(_gte, gteBuffer, gte) + gte = new std::string(_gteCh_, _gteSz_); + delete _gteCh_; if (!reverse) { - if (start != NULL) { - DisposeStringOrBufferFromSlice(startHandle, *start); - delete start; + if (startStr != NULL) { + delete[] startStr; + startStr = NULL; } - start = new leveldb::Slice(_gte.data(), _gte.size()); - startHandle = optionsObj->Get(Nan::New("gte").ToLocalChecked()).As(); - } else { - DisposeStringOrBufferFromSlice(gteBuffer, _gte); + if (start != NULL) + delete start; + start = new leveldb::Slice(gte->data(), gte->size()); } } } @@ -526,7 +523,6 @@ NAN_METHOD(Iterator::New) { , fillCache , keyAsBuffer , valueAsBuffer - , startHandle , highWaterMark ); iterator->Wrap(info.This()); diff --git a/src/iterator.h b/src/iterator.h index eeeea3cb..bed53f8d 100644 --- a/src/iterator.h +++ b/src/iterator.h @@ -44,7 +44,6 @@ class Iterator : public Nan::ObjectWrap { , bool fillCache , bool keyAsBuffer , bool valueAsBuffer - , v8::Local &startHandle , size_t highWaterMark ); @@ -82,8 +81,6 @@ class Iterator : public Nan::ObjectWrap { AsyncWorker* endWorker; private: - Nan::Persistent persistentHandle; - bool Read (std::string& key, std::string& value); bool GetIterator (); diff --git a/src/leveldown.h b/src/leveldown.h index e3c16361..98c22dad 100644 --- a/src/leveldown.h +++ b/src/leveldown.h @@ -66,6 +66,25 @@ static inline void DisposeStringOrBufferFromSlice( } \ leveldb::Slice to(to ## Ch_, to ## Sz_); +#define LD_STRING_OR_BUFFER_TO_COPY(to, from, name) \ + size_t to ## Sz_; \ + char* to ## Ch_; \ + if (!from->ToObject().IsEmpty() \ + && node::Buffer::HasInstance(from->ToObject())) { \ + to ## Sz_ = node::Buffer::Length(from->ToObject()); \ + to ## Ch_ = new char[to ## Sz_]; \ + memcpy(to ## Ch_, node::Buffer::Data(from->ToObject()), to ## Sz_); \ + } else { \ + v8::Local to ## Str = from->ToString(); \ + to ## Sz_ = to ## Str->Utf8Length(); \ + to ## Ch_ = new char[to ## Sz_]; \ + to ## Str->WriteUtf8( \ + to ## Ch_ \ + , -1 \ + , NULL, v8::String::NO_NULL_TERMINATION \ + ); \ + } + #define LD_RETURN_CALLBACK_OR_ERROR(callback, msg) \ if (!callback.IsEmpty() && callback->IsFunction()) { \ v8::Local argv[] = { \