[sandbox] Fix heuristics in EPT::StartCompactingIfNeeded

The previous code incorrectly rounded down the number of entries to free
to kBlockSize (expressed in KB) instead of kEntriesPerBlock (expressed
in # of entries) to compute the start of the evacuation area. Further,
depending on the block sized used, the previous heuristics does not
necessarily guarantee that at least one full block would be evacuated.
This CL fixes both of these issues.

Bug: v8:10391
Change-Id: I5ddecd5d582bcf89e1c52df431f006889685320a
Cq-Include-Trybots: luci.v8.try:v8_linux64_heap_sandbox_dbg_ng,v8_linux_arm64_sim_heap_sandbox_dbg_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3837860Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82557}

src/sandbox/external-pointer-table.cc

@@ -175,18 +175,20 @@ void ExternalPointerTable::StartCompactingIfNeeded() {
   uint32_t current_capacity = capacity();
 
   // Current (somewhat arbitrary) heuristic: need compacting if the table is
-  // more than 1MB in size and is at least 10% empty.
+  // more than 1MB in size, is at least 10% empty, and if at least one block
+  // can be decommitted after successful compaction.
   uint32_t table_size = current_capacity * kSystemPointerSize;
   double free_ratio = static_cast<double>(freelist_size) /
                       static_cast<double>(current_capacity);
-  bool should_compact = (table_size >= 1 * MB) && (free_ratio >= 0.10);
+  uint32_t num_blocks_to_evacuate = (freelist_size / 2) / kEntriesPerBlock;
+  bool should_compact = (table_size >= 1 * MB) && (free_ratio >= 0.10) &&
+                        (num_blocks_to_evacuate >= 1);
 
   if (should_compact) {
-    uint32_t num_entries_to_free = freelist_size / 2;
-    num_entries_to_free = RoundDown(num_entries_to_free, kBlockSize);
-    DCHECK_GT(num_entries_to_free, 0);
+    uint32_t num_entries_to_evacuate =
+        num_blocks_to_evacuate * kEntriesPerBlock;
     // A non-zero value for this member indicates that compaction is running.
-    start_of_evacuation_area_ = current_capacity - num_entries_to_free;
+    start_of_evacuation_area_ = current_capacity - num_entries_to_evacuate;
   }
 }
 

src/sandbox/external-pointer-table.h

@@ -109,10 +109,13 @@ class V8_EXPORT_PRIVATE ExternalPointerTable {
   // This method is atomic and can be called from background threads.
   inline ExternalPointerHandle AllocateEntry();
 
-  // Determines the size of the freelist.
+  // Determines the number of entries currently on the freelist.
   // The freelist entries encode the freelist size and the next entry on the
   // list, so this routine fetches the first entry on the freelist and returns
   // the size encoded in it.
+  // As entries may be allocated from background threads while this method
+  // executes, its result should only be treated as an approximation of the
+  // real size.
   inline uint32_t FreelistSize();
 
   // Marks the specified entry as alive.
@@ -187,8 +190,8 @@ class V8_EXPORT_PRIVATE ExternalPointerTable {
   // Required for Isolate::CheckIsolateLayout().
   friend class Isolate;
 
-  // An external pointer table grows in blocks of this size. This is also the
-  // initial size of the table.
+  // An external pointer table grows and shrinks in blocks of this size. This
+  // is also the initial size of the table.
   static constexpr size_t kBlockSize = 16 * KB;
   static constexpr size_t kEntriesPerBlock = kBlockSize / kSystemPointerSize;
 
@@ -223,6 +226,7 @@ class V8_EXPORT_PRIVATE ExternalPointerTable {
   bool is_initialized() { return buffer_ != kNullAddress; }
 
   // Table capacity accesors.
+  // The capacity is expressed in number of entries.
   // The capacity of the table may increase during entry allocation (if the
   // table is grown) and may decrease during sweeping (if blocks at the end are
   // free). As the former may happen concurrently, the capacity can only be