[sandbox] Do not trigger GC during EPT entry allocation

Samuel Groß · V8 LUCI CQ · commit 852aa1865fd8 · 2022-11-28T13:38:49.000Z
This simplifies some call sites as they do not need to worry about stores to external pointer fields (especially lazily initialized ones) triggering GCs. This also keeps the sandbox and non-sandbox mode more consistent, as these stores will never trigger GC in non-sandbox builds. Since there must be millions of objects that own the external pointer table entries, the chances are quite high that GCs will anyway be scheduled. If not, we should instead see if we can introduce an API to only schedule incremental marking but not perform GC finalization. Bug: v8:13535 Change-Id: Ie3c82b51194746107d4b0ed61d47abf87d28ba63 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4061688 Commit-Queue: Samuel Groß <saelo@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Cr-Commit-Position: refs/heads/main@{#84508}
diff --git a/src/sandbox/external-pointer-table-inl.h b/src/sandbox/external-pointer-table-inl.h
@@ -6,6 +6,7 @@
 #define V8_SANDBOX_EXTERNAL_POINTER_TABLE_INL_H_
 
 #include "src/base/atomicops.h"
+#include "src/common/assert-scope.h"
 #include "src/sandbox/external-pointer-table.h"
 #include "src/sandbox/external-pointer.h"
 #include "src/utils/allocation.h"
@@ -75,6 +76,13 @@ ExternalPointerHandle ExternalPointerTable::AllocateAndInitializeEntry(
     Isolate* isolate, Address initial_value, ExternalPointerTag tag) {
   DCHECK(is_initialized());
 
+  // We currently don't want entry allocation to trigger garbage collection as
+  // this may cause seemingly harmless pointer field assignments to trigger
+  // garbage collection. This is especially true for lazily-initialized
+  // external pointer slots which will typically only allocate the external
+  // pointer table entry when the pointer is first set to a non-null value.
+  DisallowGarbageCollection no_gc;
+
   Freelist freelist;
   bool success = false;
   while (!success) {
diff --git a/src/sandbox/external-pointer-table.cc b/src/sandbox/external-pointer-table.cc
@@ -315,18 +315,6 @@ ExternalPointerTable::Freelist ExternalPointerTable::Grow(Isolate* isolate) {
 
   set_capacity(new_capacity);
 
-  // Schedule GC when the table's utilization crosses one of these thresholds.
-  constexpr double kGCThresholds[] = {0.5, 0.75, 0.9, 0.95, 0.99};
-  constexpr double kMaxCapacity = static_cast<double>(kMaxExternalPointers);
-  double old_utilization = static_cast<double>(old_capacity) / kMaxCapacity;
-  double new_utilization = static_cast<double>(new_capacity) / kMaxCapacity;
-  for (double threshold : kGCThresholds) {
-    if (old_utilization < threshold && new_utilization >= threshold) {
-      isolate->heap()->ReportExternalMemoryPressure();
-      break;
-    }
-  }
-
   // Build freelist bottom to top, which might be more cache friendly.
   uint32_t start = std::max<uint32_t>(old_capacity, 1);  // Skip entry zero
   uint32_t last = new_capacity - 1;
