Visit the Optimized Code Map on first call rather than closure creation.

RESUBMITTING: Bogus assert prevented chromium roll.

Visit the Optimized Code Map on first call rather than closure creation.

This is useful for escape analysis, and helps upcoming changes to
type feedback gathering.

Adding notry due to crashed builders:
NOTRY=true
BUG=

Committed: https://crrev.com/9336f4cc6d25d39a128176679a70dbd13a6d946e
Cr-Commit-Position: refs/heads/master@{#35395}

Committed: https://crrev.com/c2de9611281d6da70fe1770072ec55f2f8aaf467
Cr-Commit-Position: refs/heads/master@{#35440}

[mvstanton, 2016-04-07 15:01:52]

mvstanton@chromium.org changed reviewers:
+ bmeurer@chromium.org, mstarzinger@chromium.org

[mvstanton, 2016-04-07 15:01:52]

mvstanton@chromium.org changed required reviewers:
+ mstarzinger@chromium.org

[mvstanton, 2016-04-07 15:01:53]

Hi Michael,
Here is the code we discussed. The CL was in the tree last month as part of a
wider CL that moved the feedback vector as well. This is a clean cut of the
compile lazy work from that one. The platform code has already been reviewed by
Benedikt.

Benedikt, it's just FYI for you.
Thanks!
--Mike

[Michael Starzinger, 2016-04-08 12:44:02]

LGTM if comments are addressed. Thanks!

https://codereview.chromium.org/1670143002/diff/80001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/1670143002/diff/80001/src/compiler.cc#newcode...
src/compiler.cc:1086: result = shared->SearchOptimizedCodeMap(
nit: Just use "function->shared()" directly here, raw pointer is scary.

https://codereview.chromium.org/1670143002/diff/80001/src/compiler.cc#newcode...
src/compiler.cc:1087: *function->GetIsolate()->native_context(),
BailoutId::None());
nit: The "isolate" variable is available, let's use it for readability.

https://codereview.chromium.org/1670143002/diff/80001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/1670143002/diff/80001/src/objects-inl.h#newco...
src/objects-inl.h:5753: return FixedArray::kHeaderSize +
These can all be statically computed, can we make them just "static const int"
member in the header instead of functions?

https://codereview.chromium.org/1670143002/diff/80001/test/cctest/heap/test-h...
File test/cctest/heap/test-heap.cc (right):

https://codereview.chromium.org/1670143002/diff/80001/test/cctest/heap/test-h...
test/cctest/heap/test-heap.cc:1522: if (i::FLAG_ignition) return;
This should actually only happen because of eager compilation with Ignition. Can
we change this to "i::FLAG_ignition_eager = false" here? If that doesn't work,
then I would vote for marking the test as FAIL for Ignition in the status file,
because I think this is something we would need to investigate then.

[Michael Starzinger, 2016-04-08 12:53:06]

https://codereview.chromium.org/1670143002/diff/80001/test/cctest/heap/test-h...
File test/cctest/heap/test-heap.cc (right):

https://codereview.chromium.org/1670143002/diff/80001/test/cctest/heap/test-h...
test/cctest/heap/test-heap.cc:1522: if (i::FLAG_ignition) return;
On 2016/04/08 12:44:02, Michael Starzinger wrote:
> This should actually only happen because of eager compilation with Ignition.
Can
> we change this to "i::FLAG_ignition_eager = false" here? If that doesn't work,
> then I would vote for marking the test as FAIL for Ignition in the status
file,
> because I think this is something we would need to investigate then.

Actually, as discussed offline: This should even work with eager compilation,
because the closure should still be initialized to the CompileLazy builtin, even
if the shared function info is already compiled. So better check if this is
still needed at all. If it is, then we should mark it as FAIL for Ignitition
instead.

[mvstanton, 2016-04-11 13:36:14]

Thanks Michael, I'm following up on the layout test/native-api thing that
worried me from the previous run at this, but if that's all clean, I'll submit
to the queue.
--Mike

https://codereview.chromium.org/1670143002/diff/80001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/1670143002/diff/80001/src/compiler.cc#newcode...
src/compiler.cc:1086: result = shared->SearchOptimizedCodeMap(
On 2016/04/08 12:44:02, Michael Starzinger wrote:
> nit: Just use "function->shared()" directly here, raw pointer is scary.

Done.

https://codereview.chromium.org/1670143002/diff/80001/src/compiler.cc#newcode...
src/compiler.cc:1087: *function->GetIsolate()->native_context(),
BailoutId::None());
On 2016/04/08 12:44:02, Michael Starzinger wrote:
> nit: The "isolate" variable is available, let's use it for readability.

Done.

https://codereview.chromium.org/1670143002/diff/80001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/1670143002/diff/80001/src/objects-inl.h#newco...
src/objects-inl.h:5753: return FixedArray::kHeaderSize +
On 2016/04/08 12:44:02, Michael Starzinger wrote:
> These can all be statically computed, can we make them just "static const int"
> member in the header instead of functions?

Absolutely, much better, done.

https://codereview.chromium.org/1670143002/diff/80001/test/cctest/heap/test-h...
File test/cctest/heap/test-heap.cc (right):

https://codereview.chromium.org/1670143002/diff/80001/test/cctest/heap/test-h...
test/cctest/heap/test-heap.cc:1522: if (i::FLAG_ignition) return;
On 2016/04/08 12:44:02, Michael Starzinger wrote:
> This should actually only happen because of eager compilation with Ignition.
Can
> we change this to "i::FLAG_ignition_eager = false" here? If that doesn't work,
> then I would vote for marking the test as FAIL for Ignition in the status
file,
> because I think this is something we would need to investigate then.

Acknowledged.

https://codereview.chromium.org/1670143002/diff/80001/test/cctest/heap/test-h...
test/cctest/heap/test-heap.cc:1522: if (i::FLAG_ignition) return;
On 2016/04/08 12:53:05, Michael Starzinger wrote:
> On 2016/04/08 12:44:02, Michael Starzinger wrote:
> > This should actually only happen because of eager compilation with Ignition.
> Can
> > we change this to "i::FLAG_ignition_eager = false" here? If that doesn't
work,
> > then I would vote for marking the test as FAIL for Ignition in the status
> file,
> > because I think this is something we would need to investigate then.
> 
> Actually, as discussed offline: This should even work with eager compilation,
> because the closure should still be initialized to the CompileLazy builtin,
even
> if the shared function info is already compiled. So better check if this is
> still needed at all. If it is, then we should mark it as FAIL for Ignitition
> instead.

Yes, it still fails with --turbo and --ignition. I created a bug, initially
assigned to me (but I'll give it to you or Ross most likely) and added a TODO in
the status file.

[mvstanton, 2016-04-11 17:06:21]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2016-04-11 17:06:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1670143002/#ps120001
(title: "REBASE.")

[commit-bot: I haz the power, 2016-04-11 17:06:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1670143002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1670143002/120001

[commit-bot: I haz the power, 2016-04-11 17:27:16]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-04-11 17:28:30]

Description was changed from

==========
Visit the Optimized Code Map on first call rather than closure creation.

This is useful for escape analysis, and helps upcoming changes to
type feedback gathering.

BUG=
==========

to

==========
Visit the Optimized Code Map on first call rather than closure creation.

This is useful for escape analysis, and helps upcoming changes to
type feedback gathering.

BUG=

Committed: https://crrev.com/9336f4cc6d25d39a128176679a70dbd13a6d946e
Cr-Commit-Position: refs/heads/master@{#35395}
==========

[commit-bot: I haz the power, 2016-04-11 17:28:32]

Patchset 7 (id:??) landed as
https://crrev.com/9336f4cc6d25d39a128176679a70dbd13a6d946e
Cr-Commit-Position: refs/heads/master@{#35395}

[Michael Hablich, 2016-04-12 07:58:15]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/1878063004/ by hablich@chromium.org.

The reason for reverting is: Blocks roll. See
https://codereview.chromium.org/1877003002/ for detailed messages.

You should be able to repro this with Linux ASAN..

[mvstanton, 2016-04-13 08:07:50]

Description was changed from

==========
Visit the Optimized Code Map on first call rather than closure creation.

This is useful for escape analysis, and helps upcoming changes to
type feedback gathering.

BUG=

Committed: https://crrev.com/9336f4cc6d25d39a128176679a70dbd13a6d946e
Cr-Commit-Position: refs/heads/master@{#35395}
==========

to

==========
RESUBMITTING: Bogus assert prevented chromium roll.

Visit the Optimized Code Map on first call rather than closure creation.

This is useful for escape analysis, and helps upcoming changes to
type feedback gathering.

BUG=

Committed: https://crrev.com/9336f4cc6d25d39a128176679a70dbd13a6d946e
Cr-Commit-Position: refs/heads/master@{#35395}
==========

[mvstanton, 2016-04-13 08:09:49]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2016-04-13 08:09:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1670143002/#ps160001
(title: "Removed overly-restrictive assert.")

[commit-bot: I haz the power, 2016-04-13 08:09:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1670143002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1670143002/160001

[commit-bot: I haz the power, 2016-04-13 08:13:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-13 08:13:10]

Try jobs failed on following builders:
  v8_linux_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/4163)

[mvstanton, 2016-04-13 10:35:23]

The CQ bit was checked by mvstanton@chromium.org

[commit-bot: I haz the power, 2016-04-13 10:35:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1670143002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1670143002/160001

[mvstanton, 2016-04-13 10:47:50]

Description was changed from

==========
RESUBMITTING: Bogus assert prevented chromium roll.

Visit the Optimized Code Map on first call rather than closure creation.

This is useful for escape analysis, and helps upcoming changes to
type feedback gathering.

BUG=

Committed: https://crrev.com/9336f4cc6d25d39a128176679a70dbd13a6d946e
Cr-Commit-Position: refs/heads/master@{#35395}
==========

to

==========
RESUBMITTING: Bogus assert prevented chromium roll.

Visit the Optimized Code Map on first call rather than closure creation.

This is useful for escape analysis, and helps upcoming changes to
type feedback gathering.

Adding notry due to crashed builders:
NOTRY=true
BUG=

Committed: https://crrev.com/9336f4cc6d25d39a128176679a70dbd13a6d946e
Cr-Commit-Position: refs/heads/master@{#35395}
==========

[commit-bot: I haz the power, 2016-04-13 10:54:35]

Description was changed from

==========
RESUBMITTING: Bogus assert prevented chromium roll.

Visit the Optimized Code Map on first call rather than closure creation.

This is useful for escape analysis, and helps upcoming changes to
type feedback gathering.

Adding notry due to crashed builders:
NOTRY=true
BUG=

Committed: https://crrev.com/9336f4cc6d25d39a128176679a70dbd13a6d946e
Cr-Commit-Position: refs/heads/master@{#35395}
==========

to

==========
RESUBMITTING: Bogus assert prevented chromium roll.

Visit the Optimized Code Map on first call rather than closure creation.

This is useful for escape analysis, and helps upcoming changes to
type feedback gathering.

Adding notry due to crashed builders:
NOTRY=true
BUG=

Committed: https://crrev.com/9336f4cc6d25d39a128176679a70dbd13a6d946e
Cr-Commit-Position: refs/heads/master@{#35395}
==========

[commit-bot: I haz the power, 2016-04-13 10:54:36]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-04-13 10:55:44]

Description was changed from

==========
RESUBMITTING: Bogus assert prevented chromium roll.

Visit the Optimized Code Map on first call rather than closure creation.

This is useful for escape analysis, and helps upcoming changes to
type feedback gathering.

Adding notry due to crashed builders:
NOTRY=true
BUG=

Committed: https://crrev.com/9336f4cc6d25d39a128176679a70dbd13a6d946e
Cr-Commit-Position: refs/heads/master@{#35395}
==========

to

==========
RESUBMITTING: Bogus assert prevented chromium roll.

Visit the Optimized Code Map on first call rather than closure creation.

This is useful for escape analysis, and helps upcoming changes to
type feedback gathering.

Adding notry due to crashed builders:
NOTRY=true
BUG=

Committed: https://crrev.com/9336f4cc6d25d39a128176679a70dbd13a6d946e
Cr-Commit-Position: refs/heads/master@{#35395}

Committed: https://crrev.com/c2de9611281d6da70fe1770072ec55f2f8aaf467
Cr-Commit-Position: refs/heads/master@{#35440}
==========

[commit-bot: I haz the power, 2016-04-13 10:55:45]

Patchset 9 (id:??) landed as
https://crrev.com/c2de9611281d6da70fe1770072ec55f2f8aaf467
Cr-Commit-Position: refs/heads/master@{#35440}