[heap] Remove unswept bytes counter

src/heap/mark-compact.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/heap/mark-compact.h"
 
 #include "src/base/atomicops.h"
 #include "src/base/bits.h"
 #include "src/base/sys-info.h"
 #include "src/code-stubs.h"
@@ skipping 551 matching lines @@
     pending_sweeper_tasks_semaphore_.Wait();
     pending_sweeper_tasks_semaphore_.Wait();
     pending_sweeper_tasks_semaphore_.Wait();
   }
 
   ParallelSweepSpacesComplete();
   sweeping_in_progress_ = false;
   RefillFreeList(heap()->paged_space(OLD_SPACE));
   RefillFreeList(heap()->paged_space(CODE_SPACE));
   RefillFreeList(heap()->paged_space(MAP_SPACE));
-  heap()->paged_space(OLD_SPACE)->ResetUnsweptFreeBytes();
-  heap()->paged_space(CODE_SPACE)->ResetUnsweptFreeBytes();
-  heap()->paged_space(MAP_SPACE)->ResetUnsweptFreeBytes();
 
 #ifdef VERIFY_HEAP
   if (FLAG_verify_heap && !evacuation()) {
     VerifyEvacuation(heap_);
   }
 #endif
 }
 
 
 bool MarkCompactCollector::IsSweepingCompleted() {
@@ skipping 14 matching lines @@
   } else if (space == heap()->code_space()) {
     free_list = free_list_code_space_.get();
   } else if (space == heap()->map_space()) {
     free_list = free_list_map_space_.get();
   } else {
     // Any PagedSpace might invoke RefillFreeLists, so we need to make sure
     // to only refill them for the old space.
     return;
   }
 
-  intptr_t freed_bytes = space->free_list()->Concatenate(free_list);
-  space->AddToAccountingStats(freed_bytes);
-  space->DecrementUnsweptFreeBytes(freed_bytes);
+  intptr_t added = space->free_list()->Concatenate(free_list);
+  space->accounting_stats_.IncreaseCapacity(added);
 }
 
 
 void Marking::TransferMark(Heap* heap, Address old_start, Address new_start) {
   // This is only used when resizing an object.
   DCHECK(MemoryChunk::FromAddress(old_start) ==
          MemoryChunk::FromAddress(new_start));
 
   if (!heap->incremental_marking()->IsMarking()) return;
 
@@ skipping 3725 matching lines @@
       continue;
     }
 
     // One unused page is kept, all further are released before sweeping them.
     if (p->LiveBytes() == 0) {
       if (unused_page_present) {
         if (FLAG_gc_verbose) {
           PrintF("Sweeping 0x%" V8PRIxPTR " released page.\n",
                  reinterpret_cast<intptr_t>(p));
         }
-        // Adjust unswept free bytes because releasing a page expects said
+        // Adjust bytes allocated because releasing a page expects said
         // counter to be accurate for unswept pages.
-        space->IncreaseUnsweptFreeBytes(p);
+        space->accounting_stats_.AllocateBytes(p->area_size());

[Hannes Payer (out of office), 2015/10/02 06:52:09]

This seems to be non-logical. We do we account here for allocated bytes, which
are not allocated.

[Michael Lippautz, 2015/10/02 13:04:35]

I agree, the change here is mechanical though. ReleasePage has a path where it
subtracts the appropriate amount of bytes.

I would rather try to fix the accounting itself in a separate CL and keep the
changes to the swept bytes counter mechanical here.

[Hannes Payer (out of office), 2015/10/05 15:23:26]

The logic is just broken Let's fix it right away.

[Michael Lippautz, 2015/10/07 13:15:39]

Done. I removed the corresponding parts. ReleasePage() has 2 callers, and the
accounting was relevant for the case where we have not swept the page.

         space->ReleasePage(p);
         continue;
       }
       unused_page_present = true;
     }
 
     switch (sweeper) {
       case CONCURRENT_SWEEPING:
         if (!parallel_sweeping_active) {
           if (FLAG_gc_verbose) {
@@ skipping 13 matching lines @@
                   IGNORE_FREE_SPACE>(space, NULL, p, NULL);
           }
           pages_swept++;
           parallel_sweeping_active = true;
         } else {
           if (FLAG_gc_verbose) {
             PrintF("Sweeping 0x%" V8PRIxPTR " in parallel.\n",
                    reinterpret_cast<intptr_t>(p));
           }
           p->parallel_sweeping_state().SetValue(MemoryChunk::kSweepingPending);
-          space->IncreaseUnsweptFreeBytes(p);
+          int to_sweep = p->area_size() - p->LiveBytes();
+          space->accounting_stats_.ShrinkSpace(to_sweep);

[Hannes Payer (out of office), 2015/10/02 06:52:09]

Here we are going to shrink the capacity, a lot. Almost every page will be
selected for concurrent sweeping resulting in a very small capacity. Our tools
may report a completely wrong picture. CommittedMemory() reports this counter to
the statistics extensions.

[Michael Lippautz, 2015/10/02 13:04:35]

I changed Committed() and MaxCommitted() to be based on an actual committed
bytes counters. This way we
- have accurate capacity/size/waste throughout sweeping, where capacity is the
actually usable memory in a space (even during sweeping).
- can later on extend on borrowing memory in RefillFreeList.
- have accurate accounting for committed memory, including page headers. We
probably get worse fragmentation as we so far ignored the page headers.

Note that there is no invariant that committed >= capacity, as we could borrow
memory. (Also, fragmentation during compaction could be quite high as
CompactionSpaces are not considered for overall statistics.)

WDYT?

         }
         space->set_end_of_unswept_pages(p);
         break;
       case SEQUENTIAL_SWEEPING: {
         if (FLAG_gc_verbose) {
           PrintF("Sweeping 0x%" V8PRIxPTR ".\n", reinterpret_cast<intptr_t>(p));
         }
         if (space->identity() == CODE_SPACE) {
           if (FLAG_zap_code_space) {
             Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, REBUILD_SKIP_LIST,
@@ skipping 195 matching lines @@
     MarkBit mark_bit = Marking::MarkBitFrom(host);
     if (Marking::IsBlack(mark_bit)) {
       RelocInfo rinfo(pc, RelocInfo::CODE_TARGET, 0, host);
       RecordRelocSlot(&rinfo, target);
     }
   }
 }
 
 }  // namespace internal
 }  // namespace v8