Reuse empty StoreBufferBlocks.

runtime/vm/store_buffer.cc

Index: runtime/vm/store_buffer.cc
diff --git a/runtime/vm/store_buffer.cc b/runtime/vm/store_buffer.cc
index 99a1371a909c10a51ca220d247d10e178617b70e..0745886fd224054eda13ef1d6bd343b9756ad069 100644
--- a/runtime/vm/store_buffer.cc
+++ b/runtime/vm/store_buffer.cc
@@ -16,6 +16,15 @@ DEFINE_LEAF_RUNTIME_ENTRY(void, StoreBufferBlockProcess, 1, Thread* thread) {
 END_LEAF_RUNTIME_ENTRY
 
 
+StoreBuffer::List StoreBuffer::global_empty_;

[Ivan Posva, 2015/06/16 21:48:19]

This will lead to statically destructed objects which is something that can
cause issues.

Should be a pointer and allocated in InitOnce().

[koda, 2015/06/18 17:40:22]

Done.

+Mutex* StoreBuffer::global_mutex_ = NULL;
+
+
+void StoreBuffer::InitOnce() {
+  global_mutex_ = new Mutex();
+}
+
+
 StoreBuffer::StoreBuffer() : mutex_(new Mutex()) {
 }
 
@@ -27,13 +36,21 @@ StoreBuffer::~StoreBuffer() {
 
 
 void StoreBuffer::Reset() {
-  MutexLocker ml(mutex_);
-  // TODO(koda): Reuse and share empty blocks between isolates.
-  while (!full_.IsEmpty()) {
-    delete full_.Pop();
-  }
-  while (!partial_.IsEmpty()) {
-    delete partial_.Pop();
+  MutexLocker local_mutex_locker(mutex_);
+  {
+    // Empty all blocks and move them to the global cache.
+    MutexLocker global_mutex_locker(global_mutex_);
+    while (!full_.IsEmpty()) {
+      StoreBufferBlock* block = full_.Pop();
+      block->Reset();
+      global_empty_.Push(block);
+    }
+    while (!partial_.IsEmpty()) {
+      StoreBufferBlock* block = partial_.Pop();
+      block->Reset();
+      global_empty_.Push(block);
+    }
+    CheckThresholdGlobalEmpty();
   }
 }
 
@@ -49,23 +66,31 @@ StoreBufferBlock* StoreBuffer::Blocks() {
 
 void StoreBuffer::PushBlock(StoreBufferBlock* block, bool check_threshold) {
   MutexLocker ml(mutex_);

[Ivan Posva, 2015/06/16 21:48:19]

ditto from below: Reduce mutex area.

[koda, 2015/06/18 17:40:23]

Done.

-  List* list = block->IsFull() ? &full_ : &partial_;
-  list->Push(block);
+  ASSERT(block->next() == NULL);  // Should be just a single block.
+  if (block->IsFull()) {
+    full_.Push(block);
+  } else if (block->IsEmpty()) {
+    MutexLocker ml(global_mutex_);
+    global_empty_.Push(block);
+    CheckThresholdGlobalEmpty();
+  } else {
+    partial_.Push(block);
+  }
   if (check_threshold) {
-    CheckThreshold();
+    CheckThresholdNonEmpty();
   }
 }
 
 
 StoreBufferBlock* StoreBuffer::PopBlock() {
   MutexLocker ml(mutex_);
-  return (!partial_.IsEmpty()) ? partial_.Pop() : PopEmptyBlock();
+  return partial_.IsEmpty() ? PopEmptyBlock() : partial_.Pop();

[Ivan Posva, 2015/06/16 21:48:19]

The mutex does not need to be held around the popping of an empty block.

[koda, 2015/06/18 17:40:22]

Done.

 }
 
 
 StoreBufferBlock* StoreBuffer::PopEmptyBlock() {
-  // TODO(koda): Reuse and share empty blocks between isolates.
-  return new StoreBufferBlock();
+  MutexLocker ml(global_mutex_);
+  return global_empty_.IsEmpty() ? new StoreBufferBlock() : global_empty_.Pop();

[Ivan Posva, 2015/06/16 21:48:19]

The mutex does not need to be held around the malloc and initialization of the
new StoreBufferBlock.

[koda, 2015/06/18 17:40:23]

Done.

 }
 
 
@@ -94,18 +119,30 @@ StoreBufferBlock* StoreBuffer::List::PopAll() {
 
 
 void StoreBuffer::List::Push(StoreBufferBlock* block) {
+  ASSERT(block->next_ == NULL);
   block->next_ = head_;
   head_ = block;
   ++length_;
 }
 
 
-void StoreBuffer::CheckThreshold() {
-  // Schedule an interrupt if we have run over the max number of
-  // StoreBufferBlocks.
-  // TODO(koda): Pass threshold and callback in constructor. Cap total?
-  if (full_.length() > 100) {
-    Isolate::Current()->ScheduleInterrupts(Isolate::kStoreBufferInterrupt);
+void StoreBuffer::CheckThresholdNonEmpty() {
+  DEBUG_ASSERT(mutex_->IsOwnedByCurrentThread());
+  if (full_.length() + partial_.length() > kMaxNonEmpty) {
+    Isolate* isolate = Isolate::Current();
+    // Sanity check: it makes no sense to schedule the GC in another isolate.
+    // (If Isolate ever gets multiple store buffers, we should avoid this
+    // coupling by passing in an explicit callback+parameter at construction.)
+    ASSERT(isolate->store_buffer() == this);
+    isolate->ScheduleInterrupts(Isolate::kStoreBufferInterrupt);
+  }
+}
+
+
+void StoreBuffer::CheckThresholdGlobalEmpty() {
+  DEBUG_ASSERT(global_mutex_->IsOwnedByCurrentThread());
+  while (global_empty_.length() > kMaxGlobalEmpty) {
+    delete global_empty_.Pop();
   }
 }