Skip to content

Commit

Permalink
src: improve KVStore API
Browse files Browse the repository at this point in the history 
This adds `const char*` based APIs to KVStore to avoid multiple string
conversions (char -> Utf8 -> Local -> char etc.) when possible.

PR-URL: #31773
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@lundibundi @MylesBorins
lundibundi authored and MylesBorins committed Mar 9, 2020
1 parent bd75688 commit 296f35b
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 20 deletions.
2 changes: 2 additions & 0 deletions src/env.h
View file
Expand Up @@ -608,11 +608,13 @@ class KVStore {

virtual v8::MaybeLocal<v8::String> Get(v8::Isolate* isolate,
v8::Local<v8::String> key) const = 0;
virtual v8::Maybe<std::string> Get(const char* key) const = 0;
virtual void Set(v8::Isolate* isolate,
v8::Local<v8::String> key,
v8::Local<v8::String> value) = 0;
virtual int32_t Query(v8::Isolate* isolate,
v8::Local<v8::String> key) const = 0;
virtual int32_t Query(const char* key) const = 0;
virtual void Delete(v8::Isolate* isolate, v8::Local<v8::String> key) = 0;
virtual v8::Local<v8::Array> Enumerate(v8::Isolate* isolate) const = 0;

Expand Down
66 changes: 46 additions & 20 deletions src/node_env_var.cc
View file
Expand Up @@ -30,17 +30,21 @@ using v8::Value;
class RealEnvStore final : public KVStore {
public:
MaybeLocal<String> Get(Isolate* isolate, Local<String> key) const override;
Maybe<std::string> Get(const char* key) const override;
void Set(Isolate* isolate, Local<String> key, Local<String> value) override;
int32_t Query(Isolate* isolate, Local<String> key) const override;
int32_t Query(const char* key) const override;
void Delete(Isolate* isolate, Local<String> key) override;
Local<Array> Enumerate(Isolate* isolate) const override;
};

class MapKVStore final : public KVStore {
public:
MaybeLocal<String> Get(Isolate* isolate, Local<String> key) const override;
Maybe<std::string> Get(const char* key) const override;
void Set(Isolate* isolate, Local<String> key, Local<String> value) override;
int32_t Query(Isolate* isolate, Local<String> key) const override;
int32_t Query(const char* key) const override;
void Delete(Isolate* isolate, Local<String> key) override;
Local<Array> Enumerate(Isolate* isolate) const override;

Expand Down Expand Up @@ -72,26 +76,36 @@ void DateTimeConfigurationChangeNotification(Isolate* isolate, const T& key) {
}
}

MaybeLocal<String> RealEnvStore::Get(Isolate* isolate,
Local<String> property) const {
Maybe<std::string> RealEnvStore::Get(const char* key) const {
Mutex::ScopedLock lock(per_process::env_var_mutex);

node::Utf8Value key(isolate, property);
size_t init_sz = 256;
MaybeStackBuffer<char, 256> val;
int ret = uv_os_getenv(*key, *val, &init_sz);
int ret = uv_os_getenv(key, *val, &init_sz);

if (ret == UV_ENOBUFS) {
// Buffer is not large enough, reallocate to the updated init_sz
// and fetch env value again.
val.AllocateSufficientStorage(init_sz);
ret = uv_os_getenv(*key, *val, &init_sz);
ret = uv_os_getenv(key, *val, &init_sz);
}

if (ret >= 0) { // Env key value fetch success.
MaybeLocal<String> value_string =
String::NewFromUtf8(isolate, *val, NewStringType::kNormal, init_sz);
return value_string;
return v8::Just(std::string(*val, init_sz));
}

return v8::Nothing<std::string>();
}

MaybeLocal<String> RealEnvStore::Get(Isolate* isolate,
Local<String> property) const {
node::Utf8Value key(isolate, property);
Maybe<std::string> value = Get(*key);

if (value.IsJust()) {
std::string val = value.FromJust();
return String::NewFromUtf8(
isolate, val.data(), NewStringType::kNormal, val.size());
}

return MaybeLocal<String>();
Expand All @@ -112,14 +126,12 @@ void RealEnvStore::Set(Isolate* isolate,
DateTimeConfigurationChangeNotification(isolate, key);
}

int32_t RealEnvStore::Query(Isolate* isolate, Local<String> property) const {
int32_t RealEnvStore::Query(const char* key) const {
Mutex::ScopedLock lock(per_process::env_var_mutex);

node::Utf8Value key(isolate, property);

char val[2];
size_t init_sz = sizeof(val);
int ret = uv_os_getenv(*key, val, &init_sz);
int ret = uv_os_getenv(key, val, &init_sz);

if (ret == UV_ENOENT) {
return -1;
Expand All @@ -136,6 +148,11 @@ int32_t RealEnvStore::Query(Isolate* isolate, Local<String> property) const {
return 0;
}

int32_t RealEnvStore::Query(Isolate* isolate, Local<String> property) const {
node::Utf8Value key(isolate, property);
return Query(*key);
}

void RealEnvStore::Delete(Isolate* isolate, Local<String> property) {
Mutex::ScopedLock lock(per_process::env_var_mutex);

Expand Down Expand Up @@ -190,13 +207,19 @@ std::shared_ptr<KVStore> KVStore::Clone(v8::Isolate* isolate) const {
return copy;
}

MaybeLocal<String> MapKVStore::Get(Isolate* isolate, Local<String> key) const {
Maybe<std::string> MapKVStore::Get(const char* key) const {
Mutex::ScopedLock lock(mutex_);
auto it = map_.find(key);
return it == map_.end() ? v8::Nothing<std::string>() : v8::Just(it->second);
}

MaybeLocal<String> MapKVStore::Get(Isolate* isolate, Local<String> key) const {
Utf8Value str(isolate, key);
auto it = map_.find(std::string(*str, str.length()));
if (it == map_.end()) return Local<String>();
return String::NewFromUtf8(isolate, it->second.data(),
NewStringType::kNormal, it->second.size());
Maybe<std::string> value = Get(*str);
if (value.IsNothing()) return Local<String>();
std::string val = value.FromJust();
return String::NewFromUtf8(
isolate, val.data(), NewStringType::kNormal, val.size());
}

void MapKVStore::Set(Isolate* isolate, Local<String> key, Local<String> value) {
Expand All @@ -209,11 +232,14 @@ void MapKVStore::Set(Isolate* isolate, Local<String> key, Local<String> value) {
}
}

int32_t MapKVStore::Query(Isolate* isolate, Local<String> key) const {
int32_t MapKVStore::Query(const char* key) const {
Mutex::ScopedLock lock(mutex_);
return map_.find(key) == map_.end() ? -1 : 0;
}

int32_t MapKVStore::Query(Isolate* isolate, Local<String> key) const {
Utf8Value str(isolate, key);
auto it = map_.find(std::string(*str, str.length()));
return it == map_.end() ? -1 : 0;
return Query(*str);
}

void MapKVStore::Delete(Isolate* isolate, Local<String> key) {
Expand Down

0 comments on commit 296f35b

Please sign in to comment.