Refactor 'dart:profiler' UserTag API

Refactor 'dart:profiler' UserTag API

- Each isolate starts off with a 'Default' UserTag set.
- A null tag cannot be set.
- Calls to clearCurrentTag() should be replaced with a call to UserTag.defaultTag.makeCurrent().
- makeCurrent returns the previously current tag.

This allows for the following pattern to work:

callee() {
  var old = calleeTag.makeCurrent();
  ...
  old.makeCurrent();
}

caller() {
  var old = callerTag.makeCurrent();
  ...
  callee();
  assert(getCurrentTag() == callerTag);
  ...
  old.makeCurrent();
}

R=iposva@google.com, zra@google.com

Committed: https://code.google.com/p/dart/source/detail?r=35810

[Cutch, 2014-05-06 14:54:59]

+ zra for ARM/MIPS intrinsic changes.

[zra, 2014-05-06 15:27:24]

arm/mips lgtm

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
File runtime/vm/intrinsifier_arm.cc (right):

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
runtime/vm/intrinsifier_arm.cc:1727: __ ldr(R2, Address(R1,
Isolate::current_tag_offset()));
Could you load this directly into R0, and get rid of the mov into R0 before the
return?

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
File runtime/vm/intrinsifier_ia32.cc (right):

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
runtime/vm/intrinsifier_ia32.cc:1753: __ movl(EBX, current_tag_addr);
Here, too?

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
File runtime/vm/intrinsifier_mips.cc (right):

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
runtime/vm/intrinsifier_mips.cc:1787: __ lw(T2, Address(T1,
Isolate::current_tag_offset()));
Same here.

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
File runtime/vm/intrinsifier_x64.cc (right):

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
runtime/vm/intrinsifier_x64.cc:1659: __ movq(R10, Address(RBX,
Isolate::current_tag_offset()));
Here, too?

[Ivan Posva, 2014-05-06 16:43:06]

LGTM after cleaning up the assembly.

-Ivan

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
File runtime/vm/intrinsifier_mips.cc (left):

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
runtime/vm/intrinsifier_mips.cc:1796: __ delay_slot()->LoadImmediate(V0,
reinterpret_cast<int32_t>(Object::null()));
Zach, can you please make sure this is not allowed?

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/object.h#newc...
runtime/vm/object.h:6886: static RawUserTag* MakeDefault();
As this corresponds to "static UserTag get defaultTag" I would rename it to
DefaultTag and drop the Make out of it. Otherwise it reads strange at the places
where you try to get the default tag.

https://codereview.chromium.org/266913010/diff/40001/sdk/lib/profiler/profile...
File sdk/lib/profiler/profiler.dart (right):

https://codereview.chromium.org/266913010/diff/40001/sdk/lib/profiler/profile...
sdk/lib/profiler/profiler.dart:34: for (var tag in _instances) {
How about using a HashMap here?

https://codereview.chromium.org/266913010/diff/40001/sdk/lib/profiler/profile...
sdk/lib/profiler/profiler.dart:55: return _currentTag;
Shouldn't you be returning old here?

[Cutch, 2014-05-06 17:00:24]

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
File runtime/vm/intrinsifier_arm.cc (right):

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
runtime/vm/intrinsifier_arm.cc:1727: __ ldr(R2, Address(R1,
Isolate::current_tag_offset()));
On 2014/05/06 15:27:24, zra wrote:
> Could you load this directly into R0, and get rid of the mov into R0 before
the
> return?

Done.

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
File runtime/vm/intrinsifier_ia32.cc (right):

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
runtime/vm/intrinsifier_ia32.cc:1753: __ movl(EBX, current_tag_addr);
On 2014/05/06 15:27:24, zra wrote:
> Here, too?

Done.

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
File runtime/vm/intrinsifier_mips.cc (right):

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
runtime/vm/intrinsifier_mips.cc:1787: __ lw(T2, Address(T1,
Isolate::current_tag_offset()));
On 2014/05/06 15:27:24, zra wrote:
> Same here.

Done.

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
File runtime/vm/intrinsifier_x64.cc (right):

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/intrinsifier_...
runtime/vm/intrinsifier_x64.cc:1659: __ movq(R10, Address(RBX,
Isolate::current_tag_offset()));
On 2014/05/06 15:27:24, zra wrote:
> Here, too?

Done.

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/266913010/diff/40001/runtime/vm/object.h#newc...
runtime/vm/object.h:6886: static RawUserTag* MakeDefault();
On 2014/05/06 16:43:06, Ivan Posva wrote:
> As this corresponds to "static UserTag get defaultTag" I would rename it to
> DefaultTag and drop the Make out of it. Otherwise it reads strange at the
places
> where you try to get the default tag.

Done.

https://codereview.chromium.org/266913010/diff/40001/sdk/lib/profiler/profile...
File sdk/lib/profiler/profiler.dart (right):

https://codereview.chromium.org/266913010/diff/40001/sdk/lib/profiler/profile...
sdk/lib/profiler/profiler.dart:34: for (var tag in _instances) {
On 2014/05/06 16:43:06, Ivan Posva wrote:
> How about using a HashMap here?

Done.

https://codereview.chromium.org/266913010/diff/40001/sdk/lib/profiler/profile...
sdk/lib/profiler/profiler.dart:55: return _currentTag;
On 2014/05/06 16:43:06, Ivan Posva wrote:
> Shouldn't you be returning old here?

Done.

[Cutch, 2014-05-06 17:07:43]

Committed patchset #5 manually as r35810 (presubmit successful).