Fix external size accounting for prologue weak persistent handles.

runtime/vm/scavenger.cc

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/scavenger.h"
 
 #include <algorithm>
 #include <map>
 #include <utility>
 
@@ skipping 253 matching lines @@
   intptr_t bytes_promoted_;
   RawObject* visiting_old_object_;
   bool in_scavenge_pointer_;
 
   DISALLOW_COPY_AND_ASSIGN(ScavengerVisitor);
 };
 
 
 class ScavengerWeakVisitor : public HandleVisitor {
  public:
-  explicit ScavengerWeakVisitor(Scavenger* scavenger)
+  explicit ScavengerWeakVisitor(Scavenger* scavenger,
+                                bool prologue_weak_were_strong)
       :  HandleVisitor(Isolate::Current()),
-         scavenger_(scavenger) {
+         scavenger_(scavenger),
+         prologue_weak_were_strong_(prologue_weak_were_strong) {
   }
 
   void VisitHandle(uword addr, bool is_prologue_weak) {
     FinalizablePersistentHandle* handle =
       reinterpret_cast<FinalizablePersistentHandle*>(addr);
     RawObject** p = handle->raw_addr();
-    Heap::Space before = handle->SpaceForExternal();
     if (scavenger_->IsUnreachable(p)) {
+      ASSERT(!is_prologue_weak || prologue_weak_were_strong_);
       handle->UpdateUnreachable(isolate(), is_prologue_weak);
     } else {
-      handle->UpdateRelocated(before, isolate());
+      handle->UpdateRelocated(isolate());
     }
   }
 
  private:
   Scavenger* scavenger_;
+  bool prologue_weak_were_strong_;
 
   DISALLOW_COPY_AND_ASSIGN(ScavengerWeakVisitor);
 };
 
 
 // Visitor used to verify that all old->new references have been added to the
 // StoreBuffers.
 class VerifyStoreBufferPointerVisitor : public ObjectPointerVisitor {
  public:
   VerifyStoreBufferPointerVisitor(Isolate* isolate, MemoryRegion* to)
@@ skipping 394 matching lines @@
   ScavengerVisitor visitor(isolate, this);
   Prologue(isolate, invoke_api_callbacks);
   IterateRoots(isolate, &visitor, !invoke_api_callbacks);
   int64_t start = OS::GetCurrentTimeMicros();
   ProcessToSpace(&visitor);
   int64_t middle = OS::GetCurrentTimeMicros();
   IterateWeakReferences(isolate, &visitor);
   // Done with promoted stack; restore external allocation.
   ASSERT(!PromotedStackHasMore());
   AllocateExternal(saved_external);
-  ScavengerWeakVisitor weak_visitor(this);
-  IterateWeakRoots(isolate, &weak_visitor, invoke_api_callbacks);
+  ScavengerWeakVisitor weak_visitor(this, !invoke_api_callbacks);
+  IterateWeakRoots(isolate, &weak_visitor, true);

[siva, 2014/03/11 16:35:01]

Why did you set the parameter 'visit_prologue_weak_persistent_handles' to true
unconditionally in this call to IterateWeakRoots?

[koda, 2014/03/11 17:35:45]

Because we must visit the prologue weak persistent handles to detect any
promotions. That was the core issue behind the performance degradation: that
UpdateRelocated was not being called for pwphs, causing incorrect accounting and
scavenge on every alloc.

The change to store a bit in the handle is a consequence, since at this point,
the ScavengerVisitor has already updated the raw_ pointers of the prologue
weaks, i.e., we are not observing the promotion as it happens (IsUnreachable),
as we could do for the non-prologue ones.

I added a brief comment, and named the flag, for clarity. Let me know if you
think it needs more.

   visitor.Finalize();
   ProcessWeakTables();
   int64_t end = OS::GetCurrentTimeMicros();
   heap_->RecordTime(kProcessToSpace, middle - start);
   heap_->RecordTime(kIterateWeaks, end - middle);
   Epilogue(isolate, &visitor, invoke_api_callbacks);
 
   if (FLAG_verify_after_gc) {
     OS::PrintErr("Verifying after Scavenge...");
     heap_->Verify();
@@ skipping 34 matching lines @@
 
 
 void Scavenger::FreeExternal(intptr_t size) {
   ASSERT(size >= 0);
   external_size_ -= size;
   ASSERT(external_size_ >= 0);
   end_ = Utils::Minimum(to_->end(), end_ + size);
 }
 
 }  // namespace dart