Migrate logging infrastructure Isolate->Thread

runtime/vm/thread.cc

Index: runtime/vm/thread.cc
diff --git a/runtime/vm/thread.cc b/runtime/vm/thread.cc
index 7d817d3072249ccff66e41f6ef07c2fca4890a76..f938ef078472e9079381760b3aa3cbd06d2b34c0 100644
--- a/runtime/vm/thread.cc
+++ b/runtime/vm/thread.cc
@@ -7,6 +7,7 @@
 #include "vm/growable_array.h"
 #include "vm/isolate.h"
 #include "vm/lockers.h"
+#include "vm/log.h"
 #include "vm/object.h"
 #include "vm/os_thread.h"
 #include "vm/profiler.h"
@@ -52,6 +53,8 @@ Thread::~Thread() {
   // Clear |this| from all isolate's thread registry.
   ThreadPruner pruner(this);
   Isolate::VisitIsolates(&pruner);
+  delete log_;
+  log_ = NULL;
 }
 
 
@@ -108,7 +111,8 @@ Thread::Thread(bool init_vm_constants)
       thread_interrupt_data_(NULL),
       isolate_(NULL),
       heap_(NULL),
-      store_buffer_block_(NULL) {
+      store_buffer_block_(NULL),
+      log_(new class Log()) {

[Ivan Posva, 2015/09/08 21:22:22]

I do not like the fact that we allocate this bottom Log object which collect all
log events unless they are flushed. I'd prefer if we had this at NULL until
somebody setup logging for a thread.

Why not new Log()? If we move the logic for filtering into the Log class then we
have a simple accessor for log() and do not fall into this problem.

[koda, 2015/09/09 00:26:57]

The log is flushed by Print or by the destructor of a LogBlock. Added ASSERT to
~Log.

Regarding moving all filtering into Log, see comment below.

   ClearState();
 
 #define DEFAULT_INIT(type_name, member_name, init_expr, default_init_value)    \
@@ -307,6 +311,15 @@ void Thread::set_cha(CHA* value) {
 }
 
 
+class Log* Thread::Log() const {
+  if (isolate_ != NULL && isolate_->ShouldLog()) {

[Ivan Posva, 2015/09/08 21:22:22]

This logic is now distributed across three different part. Please investigate
how to move this into the Log class only.

[koda, 2015/09/09 00:26:57]

Moved the flag checking logic from Isolate to Log.

To have everything in Log, we would update Thread::log_ whenever we enter/exit
an isolate, and only check the filter in a factory method
Log::NewLogForIsolate(Isolate*).

However, to ensure everyone's on the same page, I think we should first discuss
the value/future of log filtering with John when he returns, so I'll leave this
as is with the suggestion as a TODO for now, OK?

+    return log_;
+  } else {
+    return Log::NoOpLog();
+  }
+}
+
+
 void Thread::SetThreadInterrupter(ThreadInterruptCallback callback,
                                   void* data) {
   ASSERT(Thread::Current() == this);