Skip to content

Commit

Permalink
src: improve ToV8Value() functions
Browse files Browse the repository at this point in the history
- Cache the `isolate` value between calls
- Introduce an overload for dealing with integers/numbers
- Use the vectored `v8::Array::New` constructor + `MaybeStackBuffer`
  for faster array creation

PR-URL: nodejs#25288
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and BridgeAR committed Jan 16, 2019
1 parent a989df7 commit 448c9c6
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 17 deletions.
55 changes: 41 additions & 14 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,9 @@ inline char* UncheckedCalloc(size_t n) { return UncheckedCalloc<char>(n); }
void ThrowErrStringTooLong(v8::Isolate* isolate);

v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
const std::string& str) {
v8::Isolate* isolate = context->GetIsolate();
const std::string& str,
v8::Isolate* isolate) {
if (isolate == nullptr) isolate = context->GetIsolate();
if (UNLIKELY(str.size() >= static_cast<size_t>(v8::String::kMaxLength))) {
// V8 only has a TODO comment about adding an exception when the maximum
// string size is exceeded.
Expand All @@ -393,33 +394,33 @@ v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,

template <typename T>
v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
const std::vector<T>& vec) {
v8::Isolate* isolate = context->GetIsolate();
const std::vector<T>& vec,
v8::Isolate* isolate) {
if (isolate == nullptr) isolate = context->GetIsolate();
v8::EscapableHandleScope handle_scope(isolate);

v8::Local<v8::Array> arr = v8::Array::New(isolate, vec.size());
MaybeStackBuffer<v8::Local<v8::Value>, 128> arr(vec.size());
arr.SetLength(vec.size());
for (size_t i = 0; i < vec.size(); ++i) {
v8::Local<v8::Value> val;
if (!ToV8Value(context, vec[i]).ToLocal(&val) ||
arr->Set(context, i, val).IsNothing()) {
if (!ToV8Value(context, vec[i], isolate).ToLocal(&arr[i]))
return v8::MaybeLocal<v8::Value>();
}
}

return handle_scope.Escape(arr);
return handle_scope.Escape(v8::Array::New(isolate, arr.out(), arr.length()));
}

template <typename T, typename U>
v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
const std::unordered_map<T, U>& map) {
v8::Isolate* isolate = context->GetIsolate();
const std::unordered_map<T, U>& map,
v8::Isolate* isolate) {
if (isolate == nullptr) isolate = context->GetIsolate();
v8::EscapableHandleScope handle_scope(isolate);

v8::Local<v8::Map> ret = v8::Map::New(isolate);
for (const auto& item : map) {
v8::Local<v8::Value> first, second;
if (!ToV8Value(context, item.first).ToLocal(&first) ||
!ToV8Value(context, item.second).ToLocal(&second) ||
if (!ToV8Value(context, item.first, isolate).ToLocal(&first) ||
!ToV8Value(context, item.second, isolate).ToLocal(&second) ||
ret->Set(context, first, second).IsEmpty()) {
return v8::MaybeLocal<v8::Value>();
}
Expand All @@ -428,6 +429,32 @@ v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
return handle_scope.Escape(ret);
}

template <typename T, typename >
v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
const T& number,
v8::Isolate* isolate) {
if (isolate == nullptr) isolate = context->GetIsolate();

using Limits = std::numeric_limits<T>;
// Choose Uint32, Int32, or Double depending on range checks.
// These checks should all collapse at compile time.
if (static_cast<uint32_t>(Limits::max()) <=
std::numeric_limits<uint32_t>::max() &&
static_cast<uint32_t>(Limits::min()) >=
std::numeric_limits<uint32_t>::min() && Limits::is_exact) {
return v8::Integer::NewFromUnsigned(isolate, static_cast<uint32_t>(number));
}

if (static_cast<int32_t>(Limits::max()) <=
std::numeric_limits<int32_t>::max() &&
static_cast<int32_t>(Limits::min()) >=
std::numeric_limits<int32_t>::min() && Limits::is_exact) {
return v8::Integer::New(isolate, static_cast<int32_t>(number));
}

return v8::Number::New(isolate, static_cast<double>(number));
}

} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
15 changes: 12 additions & 3 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include <string.h>

#include <functional> // std::function
#include <limits>
#include <set>
#include <string>
#include <array>
Expand Down Expand Up @@ -519,13 +520,21 @@ using DeleteFnPtr = typename FunctionDeleter<T, function>::Pointer;
std::set<std::string> ParseCommaSeparatedSet(const std::string& in);

inline v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
const std::string& str);
const std::string& str,
v8::Isolate* isolate = nullptr);
template <typename T, typename test_for_number =
typename std::enable_if<std::numeric_limits<T>::is_specialized, bool>::type>
inline v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
const T& number,
v8::Isolate* isolate = nullptr);
template <typename T>
inline v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
const std::vector<T>& vec);
const std::vector<T>& vec,
v8::Isolate* isolate = nullptr);
template <typename T, typename U>
inline v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
const std::unordered_map<T, U>& map);
const std::unordered_map<T, U>& map,
v8::Isolate* isolate = nullptr);

// These macros expects a `Isolate* isolate` and a `Local<Context> context`
// to be in the scope.
Expand Down

0 comments on commit 448c9c6

Please sign in to comment.