Skip to content

Commit

Permalink
ToV8{Traits}: Optimize v8::Array creation when converting sequences
Browse files Browse the repository at this point in the history
Avoid the round-trips and overhead of calling
v8::Object::CreateDataProperty() for every single item in the sequence
we want to convert by convert all items first and then passing them at
once to the v8::Array::New() overload that takes an array and a length.

Change-Id: Ia13fbfd70c7b0c0ccd6a8c2bab5f70a10a0ba8b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4798290
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Auto-Submit: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
Commit-Queue: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/main@{#1187102}
  • Loading branch information
rakuco authored and Chromium LUCI CQ committed Aug 23, 2023
1 parent 00201db commit 1bed0ed
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 39 deletions.
25 changes: 6 additions & 19 deletions third_party/blink/renderer/bindings/core/v8/to_v8_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -386,30 +386,17 @@ namespace bindings {
template <typename ElementIDLType, typename VectorType>
inline v8::MaybeLocal<v8::Value> ToV8HelperSequence(ScriptState* script_state,
const VectorType& vector) {
v8::Isolate* isolate = script_state->GetIsolate();
v8::Local<v8::Array> array;
{
v8::Context::Scope context_scope(script_state->GetContext());
array = v8::Array::New(isolate, base::checked_cast<int>(vector.size()));
}
v8::Local<v8::Context> context = script_state->GetContext();
uint32_t index = 0;
typename VectorType::const_iterator end = vector.end();
for (typename VectorType::const_iterator iter = vector.begin(); iter != end;
++iter) {
Vector<v8::Local<v8::Value>> converted_vector(vector.size());
for (wtf_size_t i = 0; i < vector.size(); ++i) {
v8::Local<v8::Value> v8_value;
if (!ToV8Traits<ElementIDLType>::ToV8(script_state, *iter)
if (!ToV8Traits<ElementIDLType>::ToV8(script_state, vector[i])
.ToLocal(&v8_value)) {
return v8::MaybeLocal<v8::Value>();
}
bool is_property_created;
if (!array->CreateDataProperty(context, index++, v8_value)
.To(&is_property_created) ||
!is_property_created) {
return v8::Local<v8::Value>();
}
converted_vector[i] = v8_value;
}
return array;
return v8::Array::New(script_state->GetIsolate(), converted_vector.data(),
base::checked_cast<int>(converted_vector.size()));
}

// Helper function for IDLSequence in the case using reinterpret_cast
Expand Down
36 changes: 16 additions & 20 deletions third_party/blink/renderer/platform/bindings/to_v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

#include "base/containers/span.h"
#include "base/numerics/safe_conversions.h"
#include "base/ranges/algorithm.h"
#include "base/time/time.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/renderer/platform/bindings/callback_function_base.h"
Expand All @@ -27,6 +28,7 @@
#include "third_party/blink/renderer/platform/bindings/union_base.h"
#include "third_party/blink/renderer/platform/bindings/v8_binding.h"
#include "third_party/blink/renderer/platform/wtf/forward.h"
#include "third_party/blink/renderer/platform/wtf/wtf_size_t.h"
#include "v8/include/v8.h"

namespace blink {
Expand Down Expand Up @@ -323,26 +325,20 @@ inline v8::Local<v8::Array> ToV8SequenceInternal(
v8::Isolate* isolate) {
RUNTIME_CALL_TIMER_SCOPE(isolate,
RuntimeCallStats::CounterId::kToV8SequenceInternal);
v8::Local<v8::Array> array;
{
v8::Context::Scope context_scope(
creation_context->GetCreationContextChecked());
array = v8::Array::New(isolate, base::checked_cast<int>(sequence.size()));
}
v8::Local<v8::Context> context = isolate->GetCurrentContext();
uint32_t index = 0;
for (const auto& item : sequence) {
v8::Local<v8::Value> value = ToV8(item, array, isolate);
if (value.IsEmpty())
value = v8::Undefined(isolate);
bool created_property;
if (!array->CreateDataProperty(context, index++, value)
.To(&created_property) ||
!created_property) {
return v8::Local<v8::Array>();
}
}
return array;

Vector<v8::Local<v8::Value>> converted_sequence(
base::checked_cast<wtf_size_t>(sequence.size()));
base::ranges::transform(
sequence, converted_sequence.begin(), [&](const auto& item) {
v8::Local<v8::Value> value = ToV8(item, creation_context, isolate);
if (value.IsEmpty()) {
value = v8::Undefined(isolate);
}
return value;
});

return v8::Array::New(isolate, converted_sequence.data(),
base::checked_cast<int>(converted_sequence.size()));
}

// Nullable
Expand Down

0 comments on commit 1bed0ed

Please sign in to comment.