Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

src: improve KVStore API #31773

Closed
wants to merge 2 commits into from
Closed

src: improve KVStore API #31773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6693cfb
src: improve KVStore API
lundibundi Feb 13, 2020
d38ed6c
src: simplify node_worker.cc using new KVStore API
lundibundi Feb 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/env.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,11 +605,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
Open in desktop
Original file line number Diff line number Diff line change
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
12 changes: 4 additions & 8 deletions src/node_worker.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ using v8::Integer;
using v8::Isolate;
using v8::Local;
using v8::Locker;
using v8::Maybe;
using v8::MaybeLocal;
using v8::Null;
using v8::Number;
Expand Down Expand Up @@ -482,14 +483,9 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
if (args[1]->IsObject() || args[2]->IsArray()) {
per_isolate_opts.reset(new PerIsolateOptions());

HandleEnvOptions(
per_isolate_opts->per_env, [isolate, &env_vars](const char* name) {
MaybeLocal<String> value =
env_vars->Get(isolate, OneByteString(isolate, name));
return value.IsEmpty() ? std::string{}
: std::string(*String::Utf8Value(
isolate, value.ToLocalChecked()));
});
HandleEnvOptions(per_isolate_opts->per_env, [&env_vars](const char* name) {
return env_vars->Get(name).FromMaybe("");
});

#ifndef NODE_WITHOUT_NODE_OPTIONS
MaybeLocal<String> maybe_node_opts =
Expand Down