[objects] Cache the ExternalString's data in its resource

For external uncached strings (also called "Small External Strings")
with cacheable resources, we can cache its resource's data at the
string's creation time. This allows us to safely read the data from the
background as we wouldn't trigger a data() callback.

For more information regarding the investigation and possible proposals
see
https://docs.google.com/document/d/101eAQqFpBPWFGNJicxtdlwYShJkTOUsEuxkVVeu5Hrk/edit?usp=sharing

Bug: v8:7790, v8:11463
Change-Id: I6164092b01a6ccb525a9516f476e066b35fb1f96
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2685177
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72862}

include/v8.h

@@ -3323,7 +3323,8 @@ class V8_EXPORT String : public Name {
     ~ExternalStringResource() override = default;
 
     /**
-     * The string data from the underlying buffer.
+     * The string data from the underlying buffer. If the resource is cacheable
+     * then data() must return the same value for all invocations.
      */
     virtual const uint16_t* data() const = 0;
 
@@ -3332,8 +3333,31 @@ class V8_EXPORT String : public Name {
      */
     virtual size_t length() const = 0;
 
+    /**
+     * Returns the cached data from the underlying buffer. This method can be
+     * called only for cacheable resources (i.e. IsCacheable() == true) and only
+     * after UpdateDataCache() was called.
+     */
+    const uint16_t* cached_data() const {
+#if DEBUG
+      CheckCachedDataInvariants();
+#endif
+      return cached_data_;
+    }
+
+    /**
+     * Update {cached_data_} with the data from the underlying buffer. This can
+     * be called only for cacheable resources.
+     */
+    void UpdateDataCache();
+
    protected:
     ExternalStringResource() = default;
+
+   private:
+    void CheckCachedDataInvariants() const;
+
+    const uint16_t* cached_data_ = nullptr;
   };
 
   /**
@@ -3354,12 +3378,41 @@ class V8_EXPORT String : public Name {
      * buffer.
      */
     ~ExternalOneByteStringResource() override = default;
-    /** The string data from the underlying buffer.*/
+
+    /**
+     * The string data from the underlying buffer. If the resource is cacheable
+     * then data() must return the same value for all invocations.
+     */
     virtual const char* data() const = 0;
+
     /** The number of Latin-1 characters in the string.*/
     virtual size_t length() const = 0;
+
+    /**
+     * Returns the cached data from the underlying buffer. If the resource is
+     * uncacheable or if UpdateDataCache() was not called before, it has
+     * undefined behaviour.
+     */
+    const char* cached_data() const {
+#if DEBUG
+      CheckCachedDataInvariants();
+#endif
+      return cached_data_;
+    }
+
+    /**
+     * Update {cached_data_} with the data from the underlying buffer. This can
+     * be called only for cacheable resources.
+     */
+    void UpdateDataCache();
+
    protected:
     ExternalOneByteStringResource() = default;
+
+   private:
+    void CheckCachedDataInvariants() const;
+
+    const char* cached_data_ = nullptr;
   };
 
   /**

src/api/api.cc

@@ -5453,6 +5453,24 @@ String::ExternalStringResource* String::GetExternalStringResourceSlow() const {
   return nullptr;
 }
 
+void String::ExternalStringResource::UpdateDataCache() {
+  DCHECK(IsCacheable());
+  cached_data_ = data();
+}
+
+void String::ExternalStringResource::CheckCachedDataInvariants() const {
+  DCHECK(IsCacheable() && cached_data_ != nullptr);
+}
+
+void String::ExternalOneByteStringResource::UpdateDataCache() {
+  DCHECK(IsCacheable());
+  cached_data_ = data();
+}
+
+void String::ExternalOneByteStringResource::CheckCachedDataInvariants() const {
+  DCHECK(IsCacheable() && cached_data_ != nullptr);
+}
+
 String::ExternalStringResourceBase* String::GetExternalStringResourceBaseSlow(
     String::Encoding* encoding_out) const {
   i::DisallowGarbageCollection no_gc;

src/objects/string-inl.h

@@ -864,15 +864,23 @@ void ExternalString::DisposeResource(Isolate* isolate) {
 
 DEF_GETTER(ExternalOneByteString, resource,
            const ExternalOneByteString::Resource*) {
+  return mutable_resource();
+}
+
+DEF_GETTER(ExternalOneByteString, mutable_resource,
+           ExternalOneByteString::Resource*) {
   return reinterpret_cast<Resource*>(resource_as_address(isolate));
 }
 
 void ExternalOneByteString::update_data_cache(Isolate* isolate) {
-  if (is_uncached()) return;
   DisallowGarbageCollection no_gc;
-  WriteExternalPointerField(kResourceDataOffset, isolate,
-                            reinterpret_cast<Address>(resource()->data()),
-                            kExternalStringResourceDataTag);
+  if (is_uncached()) {
+    if (resource()->IsCacheable()) mutable_resource()->UpdateDataCache();
+  } else {
+    WriteExternalPointerField(kResourceDataOffset, isolate,
+                              reinterpret_cast<Address>(resource()->data()),
+                              kExternalStringResourceDataTag);
+  }
 }
 
 void ExternalOneByteString::SetResource(
@@ -894,6 +902,23 @@ void ExternalOneByteString::set_resource(
 
 const uint8_t* ExternalOneByteString::GetChars() {
   DisallowGarbageCollection no_gc;
+  if (is_uncached()) {
+    if (resource()->IsCacheable()) {
+      // TODO(solanes): Teach TurboFan/CSA to not bailout to the runtime to
+      // avoid this call.
+      return reinterpret_cast<const uint8_t*>(resource()->cached_data());
+    }
+#if DEBUG
+    // Check that this method is called only from the main thread if we have an
+    // uncached string with an uncacheable resource.
+    {
+      Isolate* isolate;
+      DCHECK_IMPLIES(GetIsolateFromHeapObject(*this, &isolate),
+                     ThreadId::Current() == isolate->thread_id());
+    }
+#endif
+  }
+
   return reinterpret_cast<const uint8_t*>(resource()->data());
 }
 
@@ -904,15 +929,23 @@ uint8_t ExternalOneByteString::Get(int index) {
 
 DEF_GETTER(ExternalTwoByteString, resource,
            const ExternalTwoByteString::Resource*) {
+  return mutable_resource();
+}
+
+DEF_GETTER(ExternalTwoByteString, mutable_resource,
+           ExternalTwoByteString::Resource*) {
   return reinterpret_cast<Resource*>(resource_as_address(isolate));
 }
 
 void ExternalTwoByteString::update_data_cache(Isolate* isolate) {
-  if (is_uncached()) return;
   DisallowGarbageCollection no_gc;
-  WriteExternalPointerField(kResourceDataOffset, isolate,
-                            reinterpret_cast<Address>(resource()->data()),
-                            kExternalStringResourceDataTag);
+  if (is_uncached()) {
+    if (resource()->IsCacheable()) mutable_resource()->UpdateDataCache();
+  } else {
+    WriteExternalPointerField(kResourceDataOffset, isolate,
+                              reinterpret_cast<Address>(resource()->data()),
+                              kExternalStringResourceDataTag);
+  }
 }
 
 void ExternalTwoByteString::SetResource(
@@ -934,6 +967,23 @@ void ExternalTwoByteString::set_resource(
 
 const uint16_t* ExternalTwoByteString::GetChars() {
   DisallowGarbageCollection no_gc;
+  if (is_uncached()) {
+    if (resource()->IsCacheable()) {
+      // TODO(solanes): Teach TurboFan/CSA to not bailout to the runtime to
+      // avoid this call.
+      return resource()->cached_data();
+    }
+#if DEBUG
+    // Check that this method is called only from the main thread if we have an
+    // uncached string with an uncacheable resource.
+    {
+      Isolate* isolate;
+      DCHECK_IMPLIES(GetIsolateFromHeapObject(*this, &isolate),
+                     ThreadId::Current() == isolate->thread_id());
+    }
+#endif
+  }
+
   return resource()->data();
 }
 

src/objects/string.h

@@ -864,6 +864,10 @@ class ExternalOneByteString : public ExternalString {
   STATIC_ASSERT(kSize == kSizeOfAllExternalStrings);
 
   OBJECT_CONSTRUCTORS(ExternalOneByteString, ExternalString);
+
+ private:
+  // The underlying resource as a non-const pointer.
+  DECL_GETTER(mutable_resource, Resource*)
 };
 
 // The ExternalTwoByteString class is an external string backed by a UTF-16
@@ -909,6 +913,10 @@ class ExternalTwoByteString : public ExternalString {
   STATIC_ASSERT(kSize == kSizeOfAllExternalStrings);
 
   OBJECT_CONSTRUCTORS(ExternalTwoByteString, ExternalString);
+
+ private:
+  // The underlying resource as a non-const pointer.
+  DECL_GETTER(mutable_resource, Resource*)
 };
 
 // A flat string reader provides random access to the contents of a