[compiler] Mark shared functions for optimization

[compiler] Mark shared functions for optimization

The current method of marking functions for optimization, which replaces
the JSFunction's code object with one that triggers optimization, would
never allow unnamed functions to be optimized. This is an issue for a
style of programming which heavily relies on passing around closures.

This patch sets a bit on the SharedFunctionInfo when a JSFunction is
marked. When another JSFunction referring to the same SharedFunctionInfo
is lazily compiled, it immediately triggers a non-concurrent optimize.

BUG=v8:5512
Committed: https://crrev.com/4a31323e973e0a03403a53c601dfd4f0237532e8
Cr-Commit-Position: refs/heads/master@{#40506}

[Leszek Swirski, 2016-10-20 13:01:35]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 13:01:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

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

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

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

Dry run: Try jobs failed on following builders:
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)

[Leszek Swirski, 2016-10-20 14:46:31]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 14:46:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-20 15:06:44]

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

[commit-bot: I haz the power, 2016-10-20 15:06:45]

Dry run: Try jobs failed on following builders:
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
  v8_win_nosnap_shared_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[Leszek Swirski, 2016-10-20 15:59:35]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 15:59:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-20 16:26:37]

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

[commit-bot: I haz the power, 2016-10-20 16:26:38]

Dry run: Try jobs failed on following builders:
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_arm_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng_trigger...)

[Leszek Swirski, 2016-10-20 16:45:24]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 16:45:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2016-10-20 16:50:40]

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

[Leszek Swirski, 2016-10-20 16:50:40]

mstarzinger@chromium.org: Please review changes in the various builtins.

mvstanton@chromium.org: Please review changes in the runtime and
SharedFunctionInfo object.

I ran this patch on octane, and it seems that (on x64) it improves Crypto and
EarleyBoyer by ~2%, and PdfJS by ~3%, which is a nice bonus. Didn't see any
regressions either, though there's bound to be some on other devices...

[commit-bot: I haz the power, 2016-10-20 17:10:08]

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

[commit-bot: I haz the power, 2016-10-20 17:10:08]

Dry run: Try jobs failed on following builders:
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/14991)
  v8_linux_dbg_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...)
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
  v8_win_nosnap_shared_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[Leszek Swirski, 2016-10-21 09:27:36]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 09:27:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2016-10-21 09:35:08]

Um, other way around, sorry.

[commit-bot: I haz the power, 2016-10-21 09:59:00]

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

[commit-bot: I haz the power, 2016-10-21 09:59:01]

Dry run: This issue passed the CQ dry run.

[mvstanton, 2016-10-21 11:14:20]

builtins change is LGTM, nice!

[Michael Starzinger, 2016-10-21 11:37:58]

https://codereview.chromium.org/2437043002/diff/100001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/2437043002/diff/100001/src/compiler.cc#newcod...
src/compiler.cc:980: if (function->shared()->was_marked_for_optimization()) {
The flag never seems to be cleared. This will mark all instances hot for all
eternity even if we deoptimized multiple times and disabled optimization. I
think we should just clear the flag here eagerly. As soon as at least one
instance within a native context has been optimized, the optimized code map
should contain an entry and the lazy compile stub will pick it up. WDYT?

[Leszek Swirski, 2016-10-21 11:40:17]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 11:40:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2016-10-21 11:44:13]

https://codereview.chromium.org/2437043002/diff/100001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/2437043002/diff/100001/src/compiler.cc#newcod...
src/compiler.cc:980: if (function->shared()->was_marked_for_optimization()) {
On 2016/10/21 11:37:58, Michael Starzinger wrote:
> The flag never seems to be cleared. This will mark all instances hot for all
> eternity even if we deoptimized multiple times and disabled optimization. I
> think we should just clear the flag here eagerly. As soon as at least one
> instance within a native context has been optimized, the optimized code map
> should contain an entry and the lazy compile stub will pick it up. WDYT?

That's a very good point, I hadn't considered optimization getting disabled.
Clearing the flag seems like a reasonable thing to do, so done.

[Michael Starzinger, 2016-10-21 11:57:15]

LGTM. Thanks!

[commit-bot: I haz the power, 2016-10-21 12:10:52]

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

[commit-bot: I haz the power, 2016-10-21 12:10:53]

Dry run: This issue passed the CQ dry run.

[Leszek Swirski, 2016-10-21 12:35:31]

The CQ bit was checked by leszeks@chromium.org

[Leszek Swirski, 2016-10-21 12:35:31]

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

[commit-bot: I haz the power, 2016-10-21 12:35:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 13:12:51]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-11-17 22:09:54]

Description was changed from

==========
[compiler] Mark shared functions for optimization

The current method of marking functions for optimization, which replaces
the JSFunction's code object with one that triggers optimization, would
never allow unnamed functions to be optimized. This is an issue for a
style of programming which heavily relies on passing around closures.

This patch sets a bit on the SharedFunctionInfo when a JSFunction is
marked. When another JSFunction referring to the same SharedFunctionInfo
is lazily compiled, it immediately triggers a non-concurrent optimize.

BUG=v8:5512
==========

to

==========
[compiler] Mark shared functions for optimization

The current method of marking functions for optimization, which replaces
the JSFunction's code object with one that triggers optimization, would
never allow unnamed functions to be optimized. This is an issue for a
style of programming which heavily relies on passing around closures.

This patch sets a bit on the SharedFunctionInfo when a JSFunction is
marked. When another JSFunction referring to the same SharedFunctionInfo
is lazily compiled, it immediately triggers a non-concurrent optimize.

BUG=v8:5512

Committed: https://crrev.com/4a31323e973e0a03403a53c601dfd4f0237532e8
Cr-Commit-Position: refs/heads/master@{#40506}
==========

[commit-bot: I haz the power, 2016-11-17 22:09:55]

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