[liveedit] deal with FCG and bytecode... at the same time.

[liveedit] deal with FCG and bytecode... at the same time.

BUG=v8:5265
Committed: https://crrev.com/63dc5a02df6501b22c11597696f965636952d080
Cr-Commit-Position: refs/heads/master@{#38482}

[Yang, 2016-08-05 10:48:41]

yangguo@chromium.org changed reviewers:
+ jgruber@chromium.org, mstarzinger@chromium.org

[jgruber, 2016-08-05 12:54:51]

lgtm with two questions mainly for my understanding.

https://codereview.chromium.org/2218873002/diff/1/src/debug/liveedit.cc
File src/debug/liveedit.cc (right):

https://codereview.chromium.org/2218873002/diff/1/src/debug/liveedit.cc#newco...
src/debug/liveedit.cc:996: shared_info->ReplaceCode(*new_code);
Why is this different from below (ReplaceCode vs. ReplaceCodeObject)?

https://codereview.chromium.org/2218873002/diff/1/src/debug/liveedit.cc#newco...
src/debug/liveedit.cc:1001: if (new_shared_info->HasBytecodeArray()) {
Does this mean that new_code->is_interpreter_trampoline_builtin()?

If yes, maybe we could DCHECK it here and ReplaceCodeObject(old_code, new_code)
outside this if branch.

[Yang, 2016-08-05 12:59:47]

https://codereview.chromium.org/2218873002/diff/1/src/debug/liveedit.cc
File src/debug/liveedit.cc (right):

https://codereview.chromium.org/2218873002/diff/1/src/debug/liveedit.cc#newco...
src/debug/liveedit.cc:996: shared_info->ReplaceCode(*new_code);
On 2016/08/05 12:54:50, jgruber wrote:
> Why is this different from below (ReplaceCode vs. ReplaceCodeObject)?

We use SharedFunctionInfo::ReplaceCode here because there were no FCG code
around. So to introduce FCG code from the new shared function info, we only have
to copy it over to the old shared function info.

We use ReplaceCodeObject below because we already had FCG code for the old
shared function info. It could be referenced in JSFunctions, so we have to find
all occurrences of the old FCG code and replace it with the new FCG code.

https://codereview.chromium.org/2218873002/diff/1/src/debug/liveedit.cc#newco...
src/debug/liveedit.cc:1001: if (new_shared_info->HasBytecodeArray()) {
On 2016/08/05 12:54:50, jgruber wrote:
> Does this mean that new_code->is_interpreter_trampoline_builtin()?
> 
> If yes, maybe we could DCHECK it here and ReplaceCodeObject(old_code,
new_code)
> outside this if branch.

No. This is not equivalent. is_interpreter_trampoline_builtin means that we will
run interpreter bytecode. HasBytecodeArray() simply means we have bytecode
around, but may not actually run it if we already tiered up. Currently, this
cannot be the case, but we are lifting this restriction.

[Yang, 2016-08-09 05:48:11]

Michael, ping :)

[Yang, 2016-08-09 09:25:06]

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

[commit-bot: I haz the power, 2016-08-09 09:25:17]

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

[Michael Starzinger, 2016-08-09 09:40:46]

LGTM. Thanks! Sorry for the delay.

[commit-bot: I haz the power, 2016-08-09 10:03:38]

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

[commit-bot: I haz the power, 2016-08-09 10:03:39]

Dry run: This issue passed the CQ dry run.

[Yang, 2016-08-09 10:54:42]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2016-08-09 10:54:46]

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-08-09 10:56:44]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-08-09 10:57:01]

Description was changed from

==========
[liveedit] deal with FCG and bytecode... at the same time.

BUG=v8:5265
==========

to

==========
[liveedit] deal with FCG and bytecode... at the same time.

BUG=v8:5265

Committed: https://crrev.com/63dc5a02df6501b22c11597696f965636952d080
Cr-Commit-Position: refs/heads/master@{#38482}
==========

[commit-bot: I haz the power, 2016-08-09 10:57:02]

Patchset 1 (id:??) landed as
https://crrev.com/63dc5a02df6501b22c11597696f965636952d080
Cr-Commit-Position: refs/heads/master@{#38482}