[liveedit] Patch changed SFIs' constant pools
When live edit patches a script, it distinguishes between 'changed' and 'unchanged' functions, and unchanged functions have their position and source script patched to the new script instead of being replaced by a new SFI. However, if a 'changed' function has an inner 'unchanged' function, it also holds a pointer to the inner function in its bytecode constant pool. This constant pool entry was not being updated for changed functions (it was for unchanged), and therefore the outer changed function would compile the redundant new function instead of the old, patched, unchanged function. This patch fixes this by patching 'changed' functions' bytecode constant pools. This is done by swapping the script and script function list position of the old new and old 'unchanged' function, rather than just setting the script (and position) on the old one, and using the new function (now pointing at the old script) to read off the old function literal id. This could also be done by reading the function_literal_id off the new function, but we are soon removing that field anyway. Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: Ib22078c06539c795b418d29a493d8224ecea182e Reviewed-on: https://chromium-review.googlesource.com/1127941 Reviewed-by: Yang Guo <yangguo@chromium.org> Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Commit-Queue: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/master@{#54321}
This commit is contained in:
parent
23dbb81d8f
commit
084d472f51
@ -1046,6 +1046,7 @@ void LiveEdit::PatchScript(Isolate* isolate, Handle<Script> script,
|
||||
if (!function_data_map.Lookup(script, mapping.first, &data)) continue;
|
||||
Handle<SharedFunctionInfo> sfi;
|
||||
if (!data->shared.ToHandle(&sfi)) continue;
|
||||
DCHECK_EQ(sfi->script(), *script);
|
||||
|
||||
isolate->compilation_cache()->Remove(sfi);
|
||||
isolate->debug()->DeoptimizeFunction(sfi);
|
||||
@ -1055,12 +1056,33 @@ void LiveEdit::PatchScript(Isolate* isolate, Handle<Script> script,
|
||||
}
|
||||
UpdatePositions(isolate, sfi, diffs);
|
||||
|
||||
MaybeObject* weak_redundant_new_sfi =
|
||||
new_script->shared_function_infos()->Get(
|
||||
mapping.second->function_literal_id());
|
||||
|
||||
sfi->set_function_literal_id(mapping.second->function_literal_id());
|
||||
sfi->set_script(*new_script);
|
||||
Handle<WeakFixedArray> new_script_sfis =
|
||||
handle(new_script->shared_function_infos(), isolate);
|
||||
new_script_sfis->Set(mapping.second->function_literal_id(),
|
||||
HeapObjectReference::Weak(*sfi));
|
||||
new_script->shared_function_infos()->Set(
|
||||
mapping.second->function_literal_id(), HeapObjectReference::Weak(*sfi));
|
||||
DCHECK_EQ(sfi->function_literal_id(),
|
||||
mapping.second->function_literal_id());
|
||||
|
||||
// Swap the now-redundant, newly compiled SFI into the old script, so that
|
||||
// we can look up the old function_literal_id using the new SFI when
|
||||
// processing changed functions.
|
||||
HeapObject* redundant_new_sfi_obj;
|
||||
if (weak_redundant_new_sfi->ToStrongOrWeakHeapObject(
|
||||
&redundant_new_sfi_obj)) {
|
||||
SharedFunctionInfo* redundant_new_sfi =
|
||||
SharedFunctionInfo::cast(redundant_new_sfi_obj);
|
||||
|
||||
redundant_new_sfi->set_function_literal_id(
|
||||
mapping.first->function_literal_id());
|
||||
redundant_new_sfi->set_script(*script);
|
||||
script->shared_function_infos()->Set(
|
||||
mapping.first->function_literal_id(),
|
||||
HeapObjectReference::Weak(redundant_new_sfi));
|
||||
}
|
||||
|
||||
if (sfi->HasUncompiledDataWithPreParsedScope()) {
|
||||
sfi->ClearPreParsedScopeData();
|
||||
@ -1096,6 +1118,7 @@ void LiveEdit::PatchScript(Isolate* isolate, Handle<Script> script,
|
||||
FunctionData* data = nullptr;
|
||||
if (!function_data_map.Lookup(new_script, mapping.second, &data)) continue;
|
||||
Handle<SharedFunctionInfo> new_sfi = data->shared.ToHandleChecked();
|
||||
DCHECK_EQ(new_sfi->script(), *new_script);
|
||||
|
||||
if (!function_data_map.Lookup(script, mapping.first, &data)) continue;
|
||||
Handle<SharedFunctionInfo> sfi;
|
||||
@ -1111,7 +1134,65 @@ void LiveEdit::PatchScript(Isolate* isolate, Handle<Script> script,
|
||||
if (!js_function->is_compiled()) continue;
|
||||
JSFunction::EnsureFeedbackVector(js_function);
|
||||
}
|
||||
|
||||
if (!new_sfi->HasBytecodeArray()) continue;
|
||||
FixedArray* constants = new_sfi->GetBytecodeArray()->constant_pool();
|
||||
for (int i = 0; i < constants->length(); ++i) {
|
||||
if (!constants->get(i)->IsSharedFunctionInfo()) continue;
|
||||
SharedFunctionInfo* inner_sfi =
|
||||
SharedFunctionInfo::cast(constants->get(i));
|
||||
if (inner_sfi->script() != *script) continue;
|
||||
|
||||
// If the inner SFI's script is the old script, then this is actually a
|
||||
// redundant new_script SFI where the old script SFI was unchanged, so we
|
||||
// swapped their scripts in the unchanged iteration. This means that we
|
||||
// have to update this changed SFI's inner SFI constant to point at the
|
||||
// old inner SFI, which has already been patched to be on the new script.
|
||||
//
|
||||
// So, we look up FunctionData using the current, newly compiled
|
||||
// inner_sfi, but the resulting FunctionData will still be referring to
|
||||
// the old, unchanged SFI.
|
||||
FunctionData* data = nullptr;
|
||||
if (!function_data_map.Lookup(inner_sfi, &data)) continue;
|
||||
Handle<SharedFunctionInfo> old_unchanged_inner_sfi =
|
||||
data->shared.ToHandleChecked();
|
||||
// Now some sanity checks. Make sure that this inner_sfi is not the
|
||||
// unchanged SFI yet...
|
||||
DCHECK_NE(*old_unchanged_inner_sfi, inner_sfi);
|
||||
// ... that the unchanged SFI has already been processed and patched to be
|
||||
// on the new script ...
|
||||
DCHECK_EQ(old_unchanged_inner_sfi->script(), *new_script);
|
||||
// ... and that the id of the unchanged SFI matches the unchanged target
|
||||
// literal's id.
|
||||
DCHECK_EQ(old_unchanged_inner_sfi->function_literal_id(),
|
||||
unchanged[data->literal]->function_literal_id());
|
||||
constants->set(i, *old_unchanged_inner_sfi);
|
||||
}
|
||||
}
|
||||
#ifdef DEBUG
|
||||
{
|
||||
// Check that all the functions in the new script are valid.
|
||||
SharedFunctionInfo::ScriptIterator it(isolate, *new_script);
|
||||
while (SharedFunctionInfo* sfi = it.Next()) {
|
||||
DCHECK_EQ(sfi->script(), *new_script);
|
||||
|
||||
if (!sfi->HasBytecodeArray()) continue;
|
||||
// Check that all the functions in this function's constant pool are also
|
||||
// on the new script, and that their id matches their index in the new
|
||||
// scripts function list.
|
||||
FixedArray* constants = sfi->GetBytecodeArray()->constant_pool();
|
||||
for (int i = 0; i < constants->length(); ++i) {
|
||||
if (!constants->get(i)->IsSharedFunctionInfo()) continue;
|
||||
SharedFunctionInfo* inner_sfi =
|
||||
SharedFunctionInfo::cast(constants->get(i));
|
||||
DCHECK_EQ(inner_sfi->script(), *new_script);
|
||||
DCHECK_EQ(inner_sfi, new_script->shared_function_infos()
|
||||
->Get(inner_sfi->function_literal_id())
|
||||
->GetHeapObject());
|
||||
}
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
||||
if (restart_frame_fp) {
|
||||
for (StackFrameIterator it(isolate); !it.done(); it.Advance()) {
|
||||
|
Loading…
Reference in New Issue
Block a user