Create snapshot for get() synchronously

Has a tiny performance cost, which I negated by optimizing the
passing of options from JS to C++. The end result is faster than
before. However, I didn't check if it blocks the event loop for
a significant amount of time. Benchmarking concurrent gets might
answer that, later.

Ref: Level/community#118
Category: fix

Author: vweevers

binding.cc

@@ -189,6 +189,20 @@ static bool BooleanProperty (napi_env env, napi_value obj, const char* key,
   return DEFAULT;
 }
 
+/**
+ * Returns a boolean value.
+ * Returns 'DEFAULT' if the JS value is undefined or otherwise not a boolean.
+ */
+static bool BooleanValue (napi_env env, napi_value value, bool DEFAULT) {
+  bool result;
+
+  if (napi_get_value_bool(env, value, &result) == napi_ok) {
+    return result;
+  } else {
+    return DEFAULT;
+  }
+}
+
 enum Encoding { buffer, utf8, view };
 
 /**
@@ -211,6 +225,19 @@ static Encoding GetEncoding (napi_env env, napi_value options, const char* optio
   return Encoding::utf8;
 }
 
+/**
+ * Returns internal Encoding enum by its equivalent numeric value.
+ */
+static Encoding GetEncoding (napi_env env, napi_value value) {
+  int32_t result;
+
+  if (napi_get_value_int32(env, value, &result) == napi_ok) {
+    return static_cast<Encoding>(result);
+  }
+
+  return Encoding::utf8;
+}
+
 /**
  * Returns a uint32 property 'key' from 'obj'.
  * Returns 'DEFAULT' if the property doesn't exist.
@@ -1231,6 +1258,7 @@ struct GetWorker final : public PriorityWorker {
       key_(key),
       encoding_(encoding) {
     options_.fill_cache = fillCache;
+    options_.snapshot = database->NewSnapshot();
   }
 
   ~GetWorker () {
@@ -1239,6 +1267,7 @@ struct GetWorker final : public PriorityWorker {
 
   void DoExecute () override {
     SetStatus(database_->Get(options_, key_, value_));
+    database_->ReleaseSnapshot(options_.snapshot);
   }
 
   void HandleOKCallback (napi_env env, napi_deferred deferred) override {
@@ -1258,14 +1287,13 @@ struct GetWorker final : public PriorityWorker {
  * Gets a value from a database.
  */
 NAPI_METHOD(db_get) {
-  NAPI_ARGV(3);
+  NAPI_ARGV(4);
   NAPI_DB_CONTEXT();
   NAPI_PROMISE();
 
   leveldb::Slice key = ToSlice(env, argv[1]);
-  napi_value options = argv[2];
-  const Encoding encoding = GetEncoding(env, options, "valueEncoding");
-  const bool fillCache = BooleanProperty(env, options, "fillCache", true);
+  const Encoding encoding = GetEncoding(env, argv[2]);
+  const bool fillCache = BooleanValue(env, argv[3], true);
 
   GetWorker* worker = new GetWorker(
     env, database, deferred, key, encoding, fillCache

index.js

@@ -56,7 +56,12 @@ class ClassicLevel extends AbstractLevel {
   }
 
   async _get (key, options) {
-    return binding.db_get(this[kContext], key, options)
+    return binding.db_get(
+      this[kContext],
+      key,
+      encodingEnum(options.valueEncoding),
+      options.fillCache
+    )
   }
 
   async _getMany (keys, options) {
@@ -162,3 +167,12 @@ class ClassicLevel extends AbstractLevel {
 }
 
 exports.ClassicLevel = ClassicLevel
+
+// It's faster to read options in JS than to pass options objects to C++.
+const encodingEnum = function (encoding) {
+  if (encoding === 'buffer') return 0
+  if (encoding === 'utf8') return 1
+
+  /* istanbul ignore else: should not happen */
+  if (encoding === 'view') return 2
+}