Move vm_tags from isolate to thread, since we may have multiple threads in same isolate (GC, backgr…

Move vm_tags from isolate to thread, since we may have multiple threads in same isolate (GC, background compilation)

BUG=
R=johnmccutchan@google.com, rmacnak@google.com

Committed: https://github.com/dart-lang/sdk/commit/dc2d81b636df90bbefcf7669b7fd4bdfc45dea69

[srdjan, 2015-10-02 22:10:04]

srdjan@google.com changed reviewers:
+ johnmccutchan@google.com, rmacnak@google.com

[rmacnak, 2015-10-02 23:35:28]

lgtm

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:953: static Sample* SetupSample(Thread* thread,
We'll probably want to record whether this was a background thread.

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/stub_code_x6...
File runtime/vm/stub_code_x64.cc (right):

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/stub_code_x6...
runtime/vm/stub_code_x64.cc:49: COMPILE_ASSERT(
Unused. And below.

[Cutch, 2015-10-05 17:45:09]

LGTM with a bunch of comment nits.

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:1937: // TODO(srdjan): Discuss with John what to do here.
Currently we are only sampling the mutator thread so the isolate counters will
still be correct.

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:876: ASSERT(thread != NULL);
Move ASSERT(thread != NULL) to before the deference.

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:953: static Sample* SetupSample(Thread* thread,
On 2015/10/02 23:35:28, rmacnak wrote:
> We'll probably want to record whether this was a background thread.

Yes, we need a new 'ThreadTag' which could be:

*) Mutator
*) Collector
*) Compiler

Not needed for this CL though.

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:1000: Isolate* isolate = thread->isolate();
Maybe ASSERT(thread != NULL) first..

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/stub_code_ar...
File runtime/vm/stub_code_arm.cc (right):

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/stub_code_ar...
runtime/vm/stub_code_arm.cc:63: // Mark that the isolate is executing VM code.
s/isolate/thread here and in other comments

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/stub_code_ar...
runtime/vm/stub_code_arm.cc:94: // Mark that the isolate is executing Dart code.
nit: Mark that the "thread" is executing Dart code.

[srdjan, 2015-10-05 18:13:27]

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:1937: // TODO(srdjan): Discuss with John what to do here.
On 2015/10/05 17:45:08, Cutch wrote:
> Currently we are only sampling the mutator thread so the isolate counters will
> still be correct.

Added comment:
// Currently we are only sampling the mutator thread.

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:876: ASSERT(thread != NULL);
On 2015/10/05 17:45:09, Cutch wrote:
> Move ASSERT(thread != NULL) to before the deference.

Done.

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:1000: Isolate* isolate = thread->isolate();
On 2015/10/05 17:45:09, Cutch wrote:
> Maybe ASSERT(thread != NULL) first..

Done.

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/stub_code_ar...
File runtime/vm/stub_code_arm.cc (right):

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/stub_code_ar...
runtime/vm/stub_code_arm.cc:63: // Mark that the isolate is executing VM code.
On 2015/10/05 17:45:09, Cutch wrote:
> s/isolate/thread here and in other comments

Done.

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/stub_code_ar...
runtime/vm/stub_code_arm.cc:94: // Mark that the isolate is executing Dart code.
On 2015/10/05 17:45:09, Cutch wrote:
> nit: Mark that the "thread" is executing Dart code.

Done.

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/stub_code_x6...
File runtime/vm/stub_code_x64.cc (right):

https://codereview.chromium.org/1387643002/diff/20001/runtime/vm/stub_code_x6...
runtime/vm/stub_code_x64.cc:49: COMPILE_ASSERT(
On 2015/10/02 23:35:28, rmacnak wrote:
> Unused. And below.

Done.

[srdjan, 2015-10-05 19:50:40]

Committed patchset #4 (id:60001) manually as
dc2d81b636df90bbefcf7669b7fd4bdfc45dea69 (presubmit successful).