[rab/gsab] Decommit the memory whenever possible

Bug: v8:11111
Change-Id: Ic07628bcf6018ea9814a38a0dab3667a7d8f0d69
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3755145
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81697}

src/builtins/builtins-arraybuffer.cc

@@ -413,13 +413,6 @@ static Object ResizeHelper(BuiltinArguments args, Isolate* isolate,
                                    kMethodName)));
   }
 
-  size_t page_size = AllocatePageSize();
-  size_t new_committed_pages;
-  bool round_return_value =
-      RoundUpToPageSize(new_byte_length, page_size,
-                        JSArrayBuffer::kMaxByteLength, &new_committed_pages);
-  CHECK(round_return_value);
-
   // [RAB] Let hostHandled be ? HostResizeArrayBuffer(O, newByteLength).
   // [GSAB] Let hostHandled be ? HostGrowArrayBuffer(O, newByteLength).
   // If hostHandled is handled, return undefined.
@@ -434,8 +427,8 @@ static Object ResizeHelper(BuiltinArguments args, Isolate* isolate,
     // [RAB] NOTE: Neither creation of the new Data Block nor copying from the
     // old Data Block are observable. Implementations reserve the right to
     // implement this method as in-place growth or shrinkage.
-    if (array_buffer->GetBackingStore()->ResizeInPlace(
-            isolate, new_byte_length, new_committed_pages * page_size) !=
+    if (array_buffer->GetBackingStore()->ResizeInPlace(isolate,
+                                                       new_byte_length) !=
         BackingStore::ResizeOrGrowResult::kSuccess) {
       THROW_NEW_ERROR_RETURN_FAILURE(
           isolate, NewRangeError(MessageTemplate::kOutOfMemory,
@@ -446,8 +439,8 @@ static Object ResizeHelper(BuiltinArguments args, Isolate* isolate,
     array_buffer->set_byte_length(new_byte_length);
   } else {
     // [GSAB] (Detailed description of the algorithm omitted.)
-    auto result = array_buffer->GetBackingStore()->GrowInPlace(
-        isolate, new_byte_length, new_committed_pages * page_size);
+    auto result =
+        array_buffer->GetBackingStore()->GrowInPlace(isolate, new_byte_length);
     if (result == BackingStore::ResizeOrGrowResult::kFailure) {
       THROW_NEW_ERROR_RETURN_FAILURE(
           isolate, NewRangeError(MessageTemplate::kOutOfMemory,

src/objects/backing-store.cc

@@ -627,19 +627,47 @@ void BackingStore::UpdateSharedWasmMemoryObjects(Isolate* isolate) {
 
 // Commit already reserved memory (for RAB backing stores (not shared)).
 BackingStore::ResizeOrGrowResult BackingStore::ResizeInPlace(
-    Isolate* isolate, size_t new_byte_length, size_t new_committed_length) {
+    Isolate* isolate, size_t new_byte_length) {
+  size_t page_size = AllocatePageSize();
+  size_t new_committed_pages;
+  bool round_return_value =
+      RoundUpToPageSize(new_byte_length, page_size,
+                        JSArrayBuffer::kMaxByteLength, &new_committed_pages);
+  CHECK(round_return_value);
+
+  size_t new_committed_length = new_committed_pages * page_size;
   DCHECK_LE(new_byte_length, new_committed_length);
   DCHECK(!is_shared());
 
   if (new_byte_length < byte_length_) {
-    // TOOO(v8:11111): Figure out a strategy for shrinking - when do we
-    // un-commit the memory?
-
     // Zero the memory so that in case the buffer is grown later, we have
-    // zeroed the contents already.
+    // zeroed the contents already. This is especially needed for the portion of
+    // the memory we're not going to decommit below (since it belongs to a
+    // committed page). In addition, we don't rely on all platforms always
+    // zeroing decommitted-then-recommitted memory, but zero the memory
+    // explicitly here.
     memset(reinterpret_cast<byte*>(buffer_start_) + new_byte_length, 0,
            byte_length_ - new_byte_length);
 
+    // Check if we can un-commit some pages.
+    size_t old_committed_pages;
+    round_return_value =
+        RoundUpToPageSize(byte_length_, page_size,
+                          JSArrayBuffer::kMaxByteLength, &old_committed_pages);
+    CHECK(round_return_value);
+    DCHECK_LE(new_committed_pages, old_committed_pages);
+
+    if (new_committed_pages < old_committed_pages) {
+      size_t old_committed_length = old_committed_pages * page_size;
+      if (!i::SetPermissions(
+              GetPlatformPageAllocator(),
+              reinterpret_cast<byte*>(buffer_start_) + new_committed_length,
+              old_committed_length - new_committed_length,
+              PageAllocator::kNoAccess)) {
+        return kFailure;
+      }
+    }
+
     // Changing the byte length wouldn't strictly speaking be needed, since
     // the JSArrayBuffer already stores the updated length. This is to keep
     // the BackingStore and JSArrayBuffer in sync.
@@ -668,7 +696,15 @@ BackingStore::ResizeOrGrowResult BackingStore::ResizeInPlace(
 
 // Commit already reserved memory (for GSAB backing stores (shared)).
 BackingStore::ResizeOrGrowResult BackingStore::GrowInPlace(
-    Isolate* isolate, size_t new_byte_length, size_t new_committed_length) {
+    Isolate* isolate, size_t new_byte_length) {
+  size_t page_size = AllocatePageSize();
+  size_t new_committed_pages;
+  bool round_return_value =
+      RoundUpToPageSize(new_byte_length, page_size,
+                        JSArrayBuffer::kMaxByteLength, &new_committed_pages);
+  CHECK(round_return_value);
+
+  size_t new_committed_length = new_committed_pages * page_size;
   DCHECK_LE(new_byte_length, new_committed_length);
   DCHECK(is_shared());
   // See comment in GrowWasmMemoryInPlace.

src/objects/backing-store.h

@@ -105,10 +105,8 @@ class V8_EXPORT_PRIVATE BackingStore : public BackingStoreBase {
 
   enum ResizeOrGrowResult { kSuccess, kFailure, kRace };
 
-  ResizeOrGrowResult ResizeInPlace(Isolate* isolate, size_t new_byte_length,
-                                   size_t new_committed_length);
-  ResizeOrGrowResult GrowInPlace(Isolate* isolate, size_t new_byte_length,
-                                 size_t new_committed_length);
+  ResizeOrGrowResult ResizeInPlace(Isolate* isolate, size_t new_byte_length);
+  ResizeOrGrowResult GrowInPlace(Isolate* isolate, size_t new_byte_length);
 
   bool CanReallocate() const {
     return !is_wasm_memory_ && !custom_deleter_ && !globally_registered_ &&

test/mjsunit/resizablearraybuffer-growablesharedarraybuffer.js

@@ -608,3 +608,43 @@ const arrayBufferCtors = [[ArrayBuffer, (b) => b.resizable],
   assertEquals(4, sliced.byteLength);
   assertEquals([1, 1, 0, 0], ToNumbers(new Uint8Array(sliced)));
 })();
+
+(function DecommitMemory() {
+  const pageSize = 4096;
+  const rab = new ArrayBuffer(6 * pageSize, {maxByteLength: 12 * pageSize});
+  const ta = new Uint8Array(rab);
+  for (let i = 0; i < 6 * pageSize; ++i) {
+    ta[i] = 1;
+  }
+  for (let i = 0; i < 6 * pageSize; ++i) {
+    assertEquals(1, ta[i]);
+  }
+
+  // This resize decommits.
+  rab.resize(2 * pageSize);
+
+  for (let i = 0; i < 2 * pageSize; ++i) {
+    assertEquals(1, ta[i]);
+  }
+
+  for (let i = 2 * pageSize; i < 6 * pageSize; ++i) {
+    assertEquals(undefined, ta[i]);
+  }
+
+  // Test that the pages get re-committed and can be used normally.
+  rab.resize(12 * pageSize);
+
+  for (let i = 0; i < 2 * pageSize; ++i) {
+    assertEquals(1, ta[i]);
+  }
+  // Test that the decommitted and then again committed memory is zeroed.
+  for (let i = 2 * pageSize; i < 12 * pageSize; ++i) {
+    assertEquals(0, ta[i]);
+  }
+  for (let i = 0; i < 12 * pageSize; ++i) {
+    ta[i] = 2;
+  }
+  for (let i = 0; i < 12 * pageSize; ++i) {
+    assertEquals(2, ta[i]);
+  }
+})();