File _service:obs_scm:REVERT-use-v8-Array-Iterate-fo... of Package ungoogled-chromium

Overview Repositories Revisions Requests Users Attributes Meta

File _service:obs_scm:REVERT-use-v8-Array-Iterate-for-converting-script-wrappables.patch of Package ungoogled-chromium (Revision 4d6b29ae4fd815ce7f618bc53a9aad79)

Currently displaying revision 4d6b29ae4fd815ce7f618bc53a9aad79 , Show latest

From ce71348a09f6689dd01a68db64b172191d0182d8 Mon Sep 17 00:00:00 2001
From: Andrey Kosyakov <caseq@chromium.org>
Date: Thu, 21 Dec 2023 18:38:38 +0000
Subject: [PATCH] [bindings] Use v8::Array::Iterate for converting script
 wrappables

This changes CreateIDLSequenceFromV8Array to use the new
v8::Array::Iterate() operation.
This speeds up the "execBundles" part of the microbenchmark
at crbug.com/dawn/1858 by around 3x.
This depends on crrev.com/c/4846594 landing (and rolling) first.

This is a slight re-work of https://crrev.com/c/4847447/3,
originally by jkummerow@chromium.org

Bug: v8:14218, dawn:1858, 1511239
Change-Id: Ia266556d05b4d53e6942e12609d1c08882b4ff0f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5132129
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1240236}
---
 .../bindings/core/v8/native_value_traits.h    |  6 ++
 .../core/v8/native_value_traits_impl.h        | 91 ++++++++++++++++++-
 2 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/third_party/blink/renderer/bindings/core/v8/native_value_traits.h b/third_party/blink/renderer/bindings/core/v8/native_value_traits.h
index 1e5a0790df6d..a5c28b37e945 100644
--- a/third_party/blink/renderer/bindings/core/v8/native_value_traits.h
+++ b/third_party/blink/renderer/bindings/core/v8/native_value_traits.h
@@ -84,6 +84,12 @@ struct NativeValueTraitsBase {
       std::is_pointer_v<ImplType> ||
       requires(ImplType value) { value.IsNull(); };
 
+  // This should only be true for certain subclasses of ScriptWrappable
+  // that satisfy the assumptions of CreateIDLSequenceFromV8ArraySlow() with
+  // regards to how NativeValue() is implemented for the underlying type.
+  static constexpr bool supports_scriptwrappable_specific_fast_array_iteration =
+      false;
+
   template <typename... ExtraArgs>
   static decltype(auto) ArgumentValue(v8::Isolate* isolate,
                                       int argument_index,
diff --git a/third_party/blink/renderer/bindings/core/v8/native_value_traits_impl.h b/third_party/blink/renderer/bindings/core/v8/native_value_traits_impl.h
index 5011503dcf1c..f085b6e90516 100644
--- a/third_party/blink/renderer/bindings/core/v8/native_value_traits_impl.h
+++ b/third_party/blink/renderer/bindings/core/v8/native_value_traits_impl.h
@@ -1037,10 +1037,86 @@ CreateIDLSequenceFromV8ArraySlow(v8::Isolate* isolate,
     return {};
   }
 
-  typename NativeValueTraits<IDLSequence<T>>::ImplType result;
+  using ResultType = typename NativeValueTraits<IDLSequence<T>>::ImplType;
+  ResultType result;
   result.ReserveInitialCapacity(length);
   v8::Local<v8::Context> current_context = isolate->GetCurrentContext();
   v8::TryCatch try_block(isolate);
+
+  // Fast path -- we're creating a sequence of script wrappables, which can be
+  // done by directly getting underlying object as long as array types are
+  // homogeneous. With ScriptWrappables, we don't expect to enter JS during
+  // iteration, so we can rely on v8::Array::Iterate() which is much faster than
+  // iterating an array on the client side of the v8. Additionally, for most
+  // subsptyes of ScriptWrappables, we can speed up type checks (see more on
+  // that below next to supports_scriptwrappable_specific_fast_array_iteration
+  // check.
+  if constexpr (std::is_base_of_v<ScriptWrappable, T>) {
+    struct CallbackData {
+      STACK_ALLOCATED();
+
+     public:
+      v8::Isolate* isolate;
+      v8::TypecheckWitness witness;
+      ResultType& result;
+      ExceptionState& exception_state;
+      CallbackData(v8::Isolate* isolate,
+                   ResultType& result,
+                   ExceptionState& exception_state)
+          : isolate(isolate),
+            witness(isolate),
+            result(result),
+            exception_state(exception_state) {}
+    };
+
+    CallbackData callback_data(isolate, result, exception_state);
+    v8::Array::IterationCallback callback = [](uint32_t index,
+                                               v8::Local<v8::Value> v8_element,
+                                               void* data) {
+      CallbackData* callback_data = reinterpret_cast<CallbackData*>(data);
+      // 3.4. Initialize Si to the result of converting nextItem to an IDL value
+      //   of type T.
+      v8::TypecheckWitness& witness = callback_data->witness;
+      // We can speed up type check by taking advantage of V8's type witness,
+      // provided traits' NativeValue implementation doesn't have additional
+      // logic beyond checking the type and calling ToScriptWrappable().
+      if constexpr (
+          NativeValueTraits<
+              T>::supports_scriptwrappable_specific_fast_array_iteration) {
+        if (witness.Matches(v8_element)) {
+          auto&& value = ToScriptWrappable(v8_element.As<v8::Object>())
+                             ->template ToImpl<T>();
+          callback_data->result.push_back(std::move(value));
+          return v8::Array::CallbackResult::kContinue;
+        }
+      }
+      auto&& element = NativeValueTraits<T>::NativeValue(
+          callback_data->isolate, v8_element, callback_data->exception_state);
+      if (callback_data->exception_state.HadException()) {
+        // It doesn't matter whether we return `kException` or `kBreak` here,
+        // as that only affects the return value of `v8_array->Iterate()`,
+        // which we are ignoring.
+        return v8::Array::CallbackResult::kException;
+      }
+      if constexpr (
+          NativeValueTraits<
+              T>::supports_scriptwrappable_specific_fast_array_iteration) {
+        witness.Update(v8_element);
+      }
+      callback_data->result.push_back(std::move(element));
+      return v8::Array::CallbackResult::kContinue;
+    };
+    if (!v8_array->Iterate(current_context, callback, &callback_data)
+             .IsJust()) {
+      if (try_block.HasCaught()) {
+        exception_state.RethrowV8Exception(try_block.Exception());
+      }
+      DCHECK(exception_state.HadException());
+      return {};
+    }
+    return result;
+  }
+
   // Array length may change if array is mutated during iteration.
   for (uint32_t i = 0; i < v8_array->Length(); ++i) {
     v8::Local<v8::Value> v8_element;
@@ -1056,6 +1132,7 @@ CreateIDLSequenceFromV8ArraySlow(v8::Isolate* isolate,
       return {};
     result.push_back(std::move(element));
   }
+
   // 3.2. If next is false, then return an IDL sequence value of type
   //   sequence<T> of length i, where the value of the element at index j is Sj.
   return result;
@@ -1398,6 +1475,7 @@ struct NativeValueTraits<T> : public NativeValueTraitsBase<T*> {
   }
 };
 
+// Interface types
 template <typename T>
   requires std::derived_from<T, CallbackInterfaceBase>
 struct NativeValueTraits<IDLNullable<T>>
@@ -1470,12 +1548,21 @@ struct NativeValueTraits<T> : public NativeValueTraitsBase<T> {
 template <typename T>
   requires std::derived_from<T, ScriptWrappable>
 struct NativeValueTraits<T> : public NativeValueTraitsBase<T*> {
+  // This signifies that CreateIDLSequenceFromV8ArraySlow() may apply
+  // certain optimization based on assumptions about `NativeValue()`
+  // implementation below. For subclasses of ScriptWrappable that have
+  // different implementation of NativeValue(), this should remain false.
+  static constexpr bool supports_scriptwrappable_specific_fast_array_iteration =
+      true;
+
   static inline T* NativeValue(v8::Isolate* isolate,
                                v8::Local<v8::Value> value,
                                ExceptionState& exception_state) {
     const WrapperTypeInfo* wrapper_type_info = T::GetStaticWrapperTypeInfo();
-    if (V8PerIsolateData::From(isolate)->HasInstance(wrapper_type_info, value))
+    if (V8PerIsolateData::From(isolate)->HasInstance(wrapper_type_info,
+                                                     value)) {
       return ToScriptWrappable(value.As<v8::Object>())->template ToImpl<T>();
+    }
 
     bindings::NativeValueTraitsInterfaceNotOfType(wrapper_type_info,
                                                   exception_state);

Places

File _service:obs_scm:REVERT-use-v8-Array-Iterate-for-converting-script-wrappables.patch of Package ungoogled-chromium (Revision 4d6b29ae4fd815ce7f618bc53a9aad79)

Places