Add test of full GC on helper thread

runtime/vm/thread.cc

 // Copyright (c) 2015, 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/thread.h"
 
 #include "vm/growable_array.h"
 #include "vm/isolate.h"
 #include "vm/lockers.h"
 #include "vm/object.h"
@@ skipping 109 matching lines @@
 
 void Thread::EnterIsolate(Isolate* isolate) {
   Thread* thread = Thread::Current();
   ASSERT(thread != NULL);
   ASSERT(thread->isolate() == NULL);
   ASSERT(!isolate->HasMutatorThread());
   thread->isolate_ = isolate;
   isolate->MakeCurrentThreadMutator(thread);
   isolate->set_vm_tag(VMTag::kVMTagId);
   ASSERT(thread->store_buffer_block_ == NULL);
-  thread->store_buffer_block_ = isolate->store_buffer()->PopNonFullBlock();
+  thread->StoreBufferAcquire();
   ASSERT(isolate->heap() != NULL);
   thread->heap_ = isolate->heap();
   thread->Schedule(isolate);
   ASSERT(thread->thread_state() == NULL);
   InterruptableThreadState* thread_state =
       ThreadInterrupter::GetCurrentThreadState();
 #if defined(DEBUG)
   thread->set_thread_state(NULL);  // Exclude thread itself from the dupe check.
   Isolate::CheckForDuplicateThreadState(thread_state);
   thread->set_thread_state(thread_state);
 #endif
   ASSERT(thread_state != NULL);
   // TODO(koda): Migrate profiler interface to use Thread.
   Profiler::BeginExecution(isolate);
 }
 
 
 void Thread::ExitIsolate() {
   Thread* thread = Thread::Current();
   // TODO(koda): Audit callers; they should know whether they're in an isolate.
   if (thread == NULL || thread->isolate() == NULL) return;
   Isolate* isolate = thread->isolate();
   Profiler::EndExecution(isolate);
   thread->set_thread_state(NULL);
   thread->Unschedule();
-  StoreBufferBlock* block = thread->store_buffer_block_;
-  thread->store_buffer_block_ = NULL;
-  isolate->store_buffer()->PushBlock(block);
+  // TODO(koda): Move store_buffer_block_ into State.
+  thread->StoreBufferRelease();
   if (isolate->is_runnable()) {
     isolate->set_vm_tag(VMTag::kIdleTagId);
   } else {
     isolate->set_vm_tag(VMTag::kLoadWaitTagId);
   }
   isolate->ClearMutatorThread();
   thread->isolate_ = NULL;
   ASSERT(Isolate::Current() == NULL);
   thread->heap_ = NULL;
 }
 
 
 void Thread::EnterIsolateAsHelper(Isolate* isolate) {
   Thread* thread = Thread::Current();
   ASSERT(thread != NULL);
   ASSERT(thread->isolate() == NULL);
   thread->isolate_ = isolate;
+  ASSERT(thread->store_buffer_block_ == NULL);
+  thread->StoreBufferAcquire();
   ASSERT(isolate->heap() != NULL);
   thread->heap_ = isolate->heap();
   ASSERT(thread->thread_state() == NULL);
   // Do not update isolate->mutator_thread, but perform sanity check:
   // this thread should not be both the main mutator and helper.
   ASSERT(!isolate->MutatorThreadIsCurrentThread());
   thread->Schedule(isolate);
 }
 
 
 void Thread::ExitIsolateAsHelper() {
   Thread* thread = Thread::Current();
-  // If the helper thread chose to use the store buffer, check that it has
-  // already been flushed manually.
-  ASSERT(thread->store_buffer_block_ == NULL);
   Isolate* isolate = thread->isolate();
   ASSERT(isolate != NULL);
   thread->Unschedule();
+  // TODO(koda): Move store_buffer_block_ into State.
+  thread->StoreBufferRelease();
   thread->set_thread_state(NULL);
   thread->isolate_ = NULL;
   thread->heap_ = NULL;
   ASSERT(!isolate->MutatorThreadIsCurrentThread());
 }
 
 
 void Thread::PrepareForGC() {

[Ivan Posva, 2015/08/18 00:28:00]

Is there a reason this is a static method on the thread? Ditto for others such
as EnterIsolateAsHelper, ...

[koda, 2015/08/18 14:21:35]

It should become non-static and called from SafepointThreads, as I implied in
the TODO in the test.

Added comment.

(I originally made most public methods static to ensure they are not called from
one thread on another running (not safepointed) thread.)

   Thread* thread = Thread::Current();
-  StoreBuffer* sb = thread->isolate()->store_buffer();
-  StoreBufferBlock* block = thread->store_buffer_block_;
-  thread->store_buffer_block_ = NULL;
-  const bool kCheckThreshold = false;  // Prevent scheduling another GC.
-  sb->PushBlock(block, kCheckThreshold);
-  thread->store_buffer_block_ = sb->PopEmptyBlock();
+  const bool kDoNotCheckThreshold = false;  // Prevent scheduling another GC.
+  thread->StoreBufferRelease(/* check_threshold = */ kDoNotCheckThreshold);

[Ivan Posva, 2015/08/18 00:28:00]

Please remove the comment so it reads more naturally. Store buffer release, do
not check threshold.

[koda, 2015/08/18 14:21:35]

Done.

+  thread->store_buffer_block_ =
+      thread->isolate()->store_buffer()->PopEmptyBlock();

[Ivan Posva, 2015/08/18 00:28:00]

StoreBufferAcquire?

[koda, 2015/08/18 14:21:35]

That would not work, since we must make sure to get a completely empty block, so
that the isolate has all the entries when the GC starts. When we run all GCs on
new threads, with their own fresh store buffer blocks, it would be more natural
to leave the block as NULL here.

Added comment in the code.

 }
 
 
 void Thread::StoreBufferBlockProcess(bool check_threshold) {
-  StoreBuffer* sb = isolate()->store_buffer();
-  StoreBufferBlock* block = store_buffer_block_;
-  store_buffer_block_ = NULL;
-  sb->PushBlock(block, check_threshold);
-  store_buffer_block_ = sb->PopNonFullBlock();
+  StoreBufferRelease(check_threshold);
+  store_buffer_block_ = isolate()->store_buffer()->PopNonFullBlock();

[Ivan Posva, 2015/08/18 00:28:00]

ditto: StoreBufferAcquire.

[koda, 2015/08/18 14:21:34]

Done.

 }
 
 
 void Thread::StoreBufferAddObject(RawObject* obj) {
   store_buffer_block_->Push(obj);
   if (store_buffer_block_->IsFull()) {
     StoreBufferBlockProcess(true);
   }
 }
 
 
 void Thread::StoreBufferAddObjectGC(RawObject* obj) {
   store_buffer_block_->Push(obj);
   if (store_buffer_block_->IsFull()) {
     StoreBufferBlockProcess(false);
   }
 }
 
 
+void Thread::StoreBufferRelease(bool check_threshold) {
+  StoreBufferBlock* block = store_buffer_block_;
+  store_buffer_block_ = NULL;
+  isolate_->store_buffer()->PushBlock(block, check_threshold);
+}
+
+
+void Thread::StoreBufferAcquire() {
+  store_buffer_block_ = isolate()->store_buffer()->PopNonFullBlock();
+}
+
+
 CHA* Thread::cha() const {
   ASSERT(isolate_ != NULL);
   return isolate_->cha_;
 }
 
 
 void Thread::set_cha(CHA* value) {
   ASSERT(isolate_ != NULL);
   isolate_->cha_ = value;
 }
@@ skipping 12 matching lines @@
 #define COMPUTE_OFFSET(type_name, member_name, expr, default_init_value)       \
   ASSERT((expr)->IsVMHeapObject());                                            \
   if (object.raw() == expr) return Thread::member_name##offset();
 CACHED_VM_OBJECTS_LIST(COMPUTE_OFFSET)
 #undef COMPUTE_OFFSET
   UNREACHABLE();
   return -1;
 }
 
 }  // namespace dart