Revert "[heap] ArrayBufferTracker: Only consider committed size"

This reverts commit 6488c9e5.

Reason for revert: wasm grow memory test fails; requires investigation.

https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20shared/builds/20548/steps/Check/logs/grow-memory

Original change's description:
> [heap] ArrayBufferTracker: Only consider committed size
> 
> - Only consider commited size of ABs.
> - Compute freed memory from retained sizes byte length might be a
>   HeapNumber and thus prohibited from accessing (as it may be already
>   collected).
> 
> CQ_INCLUDE_TRYBOTS=master.tryserver.v8:v8_linux64_tsan_rel;master.tryserver.v8:v8_linux64_tsan_concurrent_marking_rel_ng;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
> 
> Bug: chromium:775896
> Change-Id: Ia0bed66afac5e4d5ed58194950a55156e19cec72
> Reviewed-on: https://chromium-review.googlesource.com/725722
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48699}

TBR=ulan@chromium.org,mlippautz@chromium.org

Change-Id: I0f8b28b876b09f149ff330e532e57cf1871e3961
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:775896
Cq-Include-Trybots: master.tryserver.v8:v8_linux64_tsan_rel;master.tryserver.v8:v8_linux64_tsan_concurrent_marking_rel_ng;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/726440Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48700}

src/heap/array-buffer-tracker-inl.h

@@ -14,9 +14,10 @@ namespace v8 {
 namespace internal {
 
 void ArrayBufferTracker::RegisterNew(Heap* heap, JSArrayBuffer* buffer) {
-  if (buffer->backing_store() == nullptr) return;
+  void* data = buffer->backing_store();
+  if (!data) return;
 
-  const size_t length = NumberToSize(buffer->byte_length());
+  size_t length = buffer->allocation_length();
   Page* page = Page::FromAddress(buffer->address());
   {
     base::LockGuard<base::RecursiveMutex> guard(page->mutex());
@@ -35,10 +36,11 @@ void ArrayBufferTracker::RegisterNew(Heap* heap, JSArrayBuffer* buffer) {
 }
 
 void ArrayBufferTracker::Unregister(Heap* heap, JSArrayBuffer* buffer) {
-  if (buffer->backing_store() == nullptr) return;
+  void* data = buffer->backing_store();
+  if (!data) return;
 
   Page* page = Page::FromAddress(buffer->address());
-  const size_t length = NumberToSize(buffer->byte_length());
+  size_t length = buffer->allocation_length();
   {
     base::LockGuard<base::RecursiveMutex> guard(page->mutex());
     LocalArrayBufferTracker* tracker = page->local_tracker();
@@ -50,25 +52,26 @@ void ArrayBufferTracker::Unregister(Heap* heap, JSArrayBuffer* buffer) {
 
 template <typename Callback>
 void LocalArrayBufferTracker::Free(Callback should_free) {
-  size_t new_retained_size = 0;
+  size_t freed_memory = 0;
+  size_t retained_size = 0;
   for (TrackingData::iterator it = array_buffers_.begin();
        it != array_buffers_.end();) {
     JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(*it);
     const size_t length = buffer->allocation_length();
     if (should_free(buffer)) {
+      freed_memory += length;
       buffer->FreeBackingStore();
       it = array_buffers_.erase(it);
     } else {
-      new_retained_size += length;
+      retained_size += length;
       ++it;
     }
   }
-  const size_t freed_memory = retained_size_ - new_retained_size;
+  retained_size_ = retained_size;
   if (freed_memory > 0) {
     heap_->update_external_memory_concurrently_freed(
         static_cast<intptr_t>(freed_memory));
   }
-  retained_size_ = new_retained_size;
 }
 
 template <typename MarkingState>

src/heap/array-buffer-tracker.cc

@@ -18,14 +18,15 @@ template <typename Callback>
 void LocalArrayBufferTracker::Process(Callback callback) {
   JSArrayBuffer* new_buffer = nullptr;
   JSArrayBuffer* old_buffer = nullptr;
-  size_t new_retained_size = 0;
-  size_t moved_size = 0;
+  size_t freed_memory = 0;
+  size_t retained_size = 0;
   for (TrackingData::iterator it = array_buffers_.begin();
        it != array_buffers_.end();) {
     old_buffer = reinterpret_cast<JSArrayBuffer*>(*it);
+    const size_t length = old_buffer->allocation_length();
     const CallbackResult result = callback(old_buffer, &new_buffer);
     if (result == kKeepEntry) {
-      new_retained_size += NumberToSize(new_buffer->byte_length());
+      retained_size += length;
       ++it;
     } else if (result == kUpdateEntry) {
       DCHECK_NOT_NULL(new_buffer);
@@ -38,25 +39,23 @@ void LocalArrayBufferTracker::Process(Callback callback) {
           tracker = target_page->local_tracker();
         }
         DCHECK_NOT_NULL(tracker);
-        const size_t size = NumberToSize(new_buffer->byte_length());
-        moved_size += size;
-        tracker->Add(new_buffer, size);
+        DCHECK_EQ(length, new_buffer->allocation_length());
+        tracker->Add(new_buffer, length);
       }
       it = array_buffers_.erase(it);
     } else if (result == kRemoveEntry) {
-      // Size of freed memory is computed to avoid looking at dead objects.
+      freed_memory += length;
       old_buffer->FreeBackingStore();
       it = array_buffers_.erase(it);
     } else {
       UNREACHABLE();
     }
   }
-  const size_t freed_memory = retained_size_ - new_retained_size - moved_size;
+  retained_size_ = retained_size;
   if (freed_memory > 0) {
     heap_->update_external_memory_concurrently_freed(
         static_cast<intptr_t>(freed_memory));
   }
-  retained_size_ = new_retained_size;
 }
 
 void ArrayBufferTracker::FreeDeadInNewSpace(Heap* heap) {

src/heap/mark-compact.cc

@@ -4107,47 +4107,32 @@ int MarkCompactCollectorBase::CollectRememberedSetUpdatingItems(
   return pages;
 }
 
-int MinorMarkCompactCollector::CollectNewSpaceArrayBufferTrackerItems(
+void MinorMarkCompactCollector::CollectNewSpaceArrayBufferTrackerItems(
     ItemParallelJob* job) {
-  int pages = 0;
   for (Page* p : new_space_evacuation_pages_) {
     if (Evacuator::ComputeEvacuationMode(p) == Evacuator::kObjectsNewToOld) {
-      if (p->local_tracker() == nullptr) continue;
-
-      pages++;
       job->AddItem(new ArrayBufferTrackerUpdatingItem(p));
     }
   }
-  return pages;
 }
 
-int MarkCompactCollector::CollectNewSpaceArrayBufferTrackerItems(
+void MarkCompactCollector::CollectNewSpaceArrayBufferTrackerItems(
     ItemParallelJob* job) {
-  int pages = 0;
   for (Page* p : new_space_evacuation_pages_) {
     if (Evacuator::ComputeEvacuationMode(p) == Evacuator::kObjectsNewToOld) {
-      if (p->local_tracker() == nullptr) continue;
-
-      pages++;
       job->AddItem(new ArrayBufferTrackerUpdatingItem(p));
     }
   }
-  return pages;
 }
 
-int MarkCompactCollector::CollectOldSpaceArrayBufferTrackerItems(
+void MarkCompactCollector::CollectOldSpaceArrayBufferTrackerItems(
     ItemParallelJob* job) {
-  int pages = 0;
   for (Page* p : old_space_evacuation_pages_) {
     if (Evacuator::ComputeEvacuationMode(p) == Evacuator::kObjectsOldToOld &&
         p->IsEvacuationCandidate()) {
-      if (p->local_tracker() == nullptr) continue;
-
-      pages++;
       job->AddItem(new ArrayBufferTrackerUpdatingItem(p));
     }
   }
-  return pages;
 }
 
 void MarkCompactCollector::UpdatePointersAfterEvacuation() {
@@ -4167,6 +4152,9 @@ void MarkCompactCollector::UpdatePointersAfterEvacuation() {
     ItemParallelJob updating_job(isolate()->cancelable_task_manager(),
                                  &page_parallel_job_semaphore_);
 
+    CollectNewSpaceArrayBufferTrackerItems(&updating_job);
+    CollectOldSpaceArrayBufferTrackerItems(&updating_job);
+
     int remembered_set_pages = 0;
     remembered_set_pages += CollectRememberedSetUpdatingItems(
         &updating_job, heap()->old_space(), RememberedSetUpdatingMode::ALL);
@@ -4188,28 +4176,20 @@ void MarkCompactCollector::UpdatePointersAfterEvacuation() {
   }
 
   {
-    // - Update pointers in map space in a separate phase to avoid data races
-    //   with Map->LayoutDescriptor edge.
-    // - Update array buffer trackers in the second phase to have access to
-    //   byte length which is potentially a HeapNumber.
+    // Update pointers in map space in a separate phase to avoid data races
+    // with Map->LayoutDescriptor edge.
     TRACE_GC(heap()->tracer(),
              GCTracer::Scope::MC_EVACUATE_UPDATE_POINTERS_SLOTS_MAP_SPACE);
     ItemParallelJob updating_job(isolate()->cancelable_task_manager(),
                                  &page_parallel_job_semaphore_);
 
-    int array_buffer_pages = 0;
-    array_buffer_pages += CollectNewSpaceArrayBufferTrackerItems(&updating_job);
-    array_buffer_pages += CollectOldSpaceArrayBufferTrackerItems(&updating_job);
-
     int remembered_set_pages = 0;
     remembered_set_pages += CollectRememberedSetUpdatingItems(
         &updating_job, heap()->map_space(), RememberedSetUpdatingMode::ALL);
-    const int remembered_set_tasks =
-        remembered_set_pages == 0
-            ? 0
-            : NumberOfParallelPointerUpdateTasks(remembered_set_pages,
-                                                 old_to_new_slots_);
-    const int num_tasks = Max(array_buffer_pages, remembered_set_tasks);
+    const int num_tasks = remembered_set_pages == 0
+                              ? 0
+                              : NumberOfParallelPointerUpdateTasks(
+                                    remembered_set_pages, old_to_new_slots_);
     if (num_tasks > 0) {
       for (int i = 0; i < num_tasks; i++) {
         updating_job.AddTask(new PointersUpdatingTask(isolate()));

src/heap/mark-compact.h

@@ -405,7 +405,7 @@ class MinorMarkCompactCollector final : public MarkCompactCollectorBase {
   UpdatingItem* CreateRememberedSetUpdatingItem(
       MemoryChunk* chunk, RememberedSetUpdatingMode updating_mode) override;
 
-  int CollectNewSpaceArrayBufferTrackerItems(ItemParallelJob* job);
+  void CollectNewSpaceArrayBufferTrackerItems(ItemParallelJob* job);
 
   int NumberOfParallelMarkingTasks(int pages);
 
@@ -897,8 +897,8 @@ class MarkCompactCollector final : public MarkCompactCollectorBase {
   UpdatingItem* CreateRememberedSetUpdatingItem(
       MemoryChunk* chunk, RememberedSetUpdatingMode updating_mode) override;
 
-  int CollectNewSpaceArrayBufferTrackerItems(ItemParallelJob* job);
-  int CollectOldSpaceArrayBufferTrackerItems(ItemParallelJob* job);
+  void CollectNewSpaceArrayBufferTrackerItems(ItemParallelJob* job);
+  void CollectOldSpaceArrayBufferTrackerItems(ItemParallelJob* job);
 
   void ReleaseEvacuationCandidates();
   void PostProcessEvacuationCandidates();