Handlify Map::CopyNormalized

Handlify Map::CopyNormalized

R=mstarzinger@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=17349

[rafaelw, 2013-10-22 02:30:54]

https://codereview.chromium.org/32483006/diff/50001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/32483006/diff/50001/src/objects.cc#newcode4451
src/objects.cc:4451: Map* fast = obj->map();
I'm assuming the cache-hit path is using raw pointers for perf reasons (which
seems scary). Let me know if this is wrong, and I'll handlify.

[Michael Starzinger, 2013-10-22 16:29:50]

https://codereview.chromium.org/32483006/diff/50001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/32483006/diff/50001/src/objects.cc#newcode4451
src/objects.cc:4451: Map* fast = obj->map();
On 2013/10/22 02:30:54, rafaelw wrote:
> I'm assuming the cache-hit path is using raw pointers for perf reasons (which
> seems scary). Let me know if this is wrong, and I'll handlify.

Yes, you are right, this is tricky. I think it gets easier if we not pull out
obj->map() into a local variable. Also, since we handlify "result" even in the
fast case already, I think it should be fine to pull up the handlification of
"result" to line 4452 here.

https://codereview.chromium.org/32483006/diff/50001/src/objects.cc#newcode4466
src/objects.cc:4466: Handle<Map> fresh = Map::CopyNormalized(fast,
SHARED_NORMALIZED_MAP);
This shouldn't compile in debug mode, as "fast" is not a handle and the "mode"
argument is missing here.

https://codereview.chromium.org/32483006/diff/50001/src/objects.cc#newcode4479
src/objects.cc:4479: return handle(Map::cast(result));
At this point "result" might be stale.

[rafaelw, 2013-10-22 18:59:58]

https://codereview.chromium.org/32483006/diff/50001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/32483006/diff/50001/src/objects.cc#newcode4451
src/objects.cc:4451: Map* fast = obj->map();
On 2013/10/22 16:29:50, Michael Starzinger wrote:
> On 2013/10/22 02:30:54, rafaelw wrote:
> > I'm assuming the cache-hit path is using raw pointers for perf reasons
(which
> > seems scary). Let me know if this is wrong, and I'll handlify.
> 
> Yes, you are right, this is tricky. I think it gets easier if we not pull out
> obj->map() into a local variable. Also, since we handlify "result" even in the
> fast case already, I think it should be fine to pull up the handlification of
> "result" to line 4452 here.

Done.

https://codereview.chromium.org/32483006/diff/50001/src/objects.cc#newcode4466
src/objects.cc:4466: Handle<Map> fresh = Map::CopyNormalized(fast,
SHARED_NORMALIZED_MAP);
DOH! Done.

On 2013/10/22 16:29:50, Michael Starzinger wrote:
> This shouldn't compile in debug mode, as "fast" is not a handle and the "mode"
> argument is missing here.

https://codereview.chromium.org/32483006/diff/50001/src/objects.cc#newcode4479
src/objects.cc:4479: return handle(Map::cast(result));
Right. Got it.

On 2013/10/22 16:29:50, Michael Starzinger wrote:
> At this point "result" might be stale.

[Michael Starzinger, 2013-10-23 08:56:18]

LGTM.

[rafaelw, 2013-10-23 19:47:44]

Committed patchset #4 manually as r17349 (presubmit successful).