Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Revert of Correctly annotate eval origin. (patchset #5 id:80001 of ht…
Browse files Browse the repository at this point in the history
…tps://codereview.chromium.org/1854713002/ )

Reason for revert:
performance impact

Original issue's description:
> Correctly annotate eval origin.
>
> There were a couple of issues with it:
> - interpreter is not supported
> - the source position was just accidentally correct for full-codegen
> - the eval origin could have been cached
>
> Also fixes a few other places to use AbstractCode.
>
> R=mstarzinger@chromium.org
>
> Committed: https://crrev.com/2f3a171adc9e620c2235bf0562145b9d4eaba66d
> Cr-Commit-Position: refs/heads/master@{#35257}
>
> Committed: https://crrev.com/ad4e8a27963b704bb70ec8bac0991c57296b1d16
> Cr-Commit-Position: refs/heads/master@{#35481}

TBR=mstarzinger@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/1888013002

Cr-Commit-Position: refs/heads/master@{#35491}
  • Loading branch information
hashseed authored and Commit bot committed Apr 14, 2016
1 parent b62af11 commit 5af0a68
Show file tree
Hide file tree
Showing 34 changed files with 145 additions and 173 deletions.
7 changes: 5 additions & 2 deletions src/accessors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -661,8 +661,11 @@ void Accessors::ScriptEvalFromScriptPositionGetter(
Script::cast(Handle<JSValue>::cast(object)->value()), isolate);
Handle<Object> result = isolate->factory()->undefined_value();
if (script->compilation_type() == Script::COMPILATION_TYPE_EVAL) {
result =
Handle<Object>(Smi::FromInt(script->eval_from_position()), isolate);
Handle<Code> code(SharedFunctionInfo::cast(
script->eval_from_shared())->code());
result = Handle<Object>(Smi::FromInt(code->SourcePosition(
script->eval_from_instructions_offset())),
isolate);
}
info.GetReturnValue().Set(Utils::ToLocal(result));
}
Expand Down
12 changes: 5 additions & 7 deletions src/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2034,7 +2034,6 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
}

i::Handle<i::Object> name_obj;
int eval_position = 0;
int line_offset = 0;
int column_offset = 0;
if (!source->resource_name.IsEmpty()) {
Expand All @@ -2047,12 +2046,11 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
column_offset = static_cast<int>(source->resource_column_offset->Value());
}
i::Handle<i::JSFunction> fun;
has_pending_exception =
!i::Compiler::GetFunctionFromEval(
source_string, outer_info, context, i::SLOPPY,
i::ONLY_SINGLE_FUNCTION_LITERAL, eval_position, line_offset,
column_offset - scope_position, name_obj, source->resource_options)
.ToHandle(&fun);
has_pending_exception = !i::Compiler::GetFunctionFromEval(
source_string, outer_info, context, i::SLOPPY,
i::ONLY_SINGLE_FUNCTION_LITERAL, line_offset,
column_offset - scope_position, name_obj,
source->resource_options).ToHandle(&fun);
if (has_pending_exception) {
isolate->ReportPendingMessages();
}
Expand Down
17 changes: 4 additions & 13 deletions src/builtins.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2068,20 +2068,11 @@ MaybeHandle<JSFunction> CompileString(Handle<Context> context,
}

// Compile source string in the native context.
StackTraceFrameIterator it(isolate);
int pos = RelocInfo::kNoPosition;
Handle<SharedFunctionInfo> outer_info;
if (!it.done() && it.is_javascript()) {
FrameSummary summary = FrameSummary::GetFirst(it.javascript_frame());
pos = summary.abstract_code()->SourcePosition(summary.code_offset());
outer_info = Handle<SharedFunctionInfo>(summary.function()->shared());
} else {
outer_info =
Handle<SharedFunctionInfo>(native_context->closure()->shared());
}

Handle<SharedFunctionInfo> outer_info(native_context->closure()->shared(),
isolate);
return Compiler::GetFunctionFromEval(source, outer_info, native_context,
SLOPPY, restriction, pos);
SLOPPY, restriction,
RelocInfo::kNoPosition);
}

} // namespace
Expand Down
15 changes: 6 additions & 9 deletions src/compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1233,9 +1233,8 @@ void Compiler::CompileForLiveEdit(Handle<Script> script) {
MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
Handle<String> source, Handle<SharedFunctionInfo> outer_info,
Handle<Context> context, LanguageMode language_mode,
ParseRestriction restriction, int eval_position, int line_offset,
int column_offset, Handle<Object> script_name,
ScriptOriginOptions options) {
ParseRestriction restriction, int line_offset, int column_offset,
Handle<Object> script_name, ScriptOriginOptions options) {
Isolate* isolate = source->GetIsolate();
int source_length = source->length();
isolate->counters()->total_eval_size()->Increment(source_length);
Expand All @@ -1244,7 +1243,7 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
CompilationCache* compilation_cache = isolate->compilation_cache();
MaybeHandle<SharedFunctionInfo> maybe_shared_info =
compilation_cache->LookupEval(source, outer_info, context, language_mode,
eval_position);
line_offset);
Handle<SharedFunctionInfo> shared_info;

Handle<Script> script;
Expand All @@ -1256,10 +1255,6 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
script->set_column_offset(column_offset);
}
script->set_origin_options(options);
script->set_compilation_type(Script::COMPILATION_TYPE_EVAL);
script->set_eval_from_shared(*outer_info);
script->set_eval_from_position(eval_position);

Zone zone(isolate->allocator());
ParseInfo parse_info(&zone, script);
CompilationInfo info(&parse_info, Handle<JSFunction>::null());
Expand All @@ -1269,6 +1264,8 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
parse_info.set_parse_restriction(restriction);
parse_info.set_context(context);

Debug::RecordEvalCaller(script);

shared_info = CompileToplevel(&info);

if (shared_info.is_null()) {
Expand All @@ -1284,7 +1281,7 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
DCHECK(is_sloppy(language_mode) ||
is_strict(shared_info->language_mode()));
compilation_cache->PutEval(source, outer_info, context, shared_info,
eval_position);
line_offset);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ class Compiler : public AllStatic {
MUST_USE_RESULT static MaybeHandle<JSFunction> GetFunctionFromEval(
Handle<String> source, Handle<SharedFunctionInfo> outer_info,
Handle<Context> context, LanguageMode language_mode,
ParseRestriction restriction, int eval_position, int line_offset = 0,
int column_offset = 0, Handle<Object> script_name = Handle<Object>(),
ParseRestriction restriction, int line_offset, int column_offset = 0,
Handle<Object> script_name = Handle<Object>(),
ScriptOriginOptions options = ScriptOriginOptions());

// Create a shared function info object for a String source within a context.
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/ast-graph-builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2456,7 +2456,7 @@ void AstGraphBuilder::VisitCall(Call* expr) {
// provide a fully resolved callee to patch into the environment.
Node* function = GetFunctionClosure();
Node* language = jsgraph()->Constant(language_mode());
Node* position = jsgraph()->Constant(expr->position());
Node* position = jsgraph()->Constant(current_scope()->start_position());
const Operator* op =
javascript()->CallRuntime(Runtime::kResolvePossiblyDirectEval);
Node* new_callee =
Expand Down
10 changes: 5 additions & 5 deletions src/debug/debug-evaluate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ MaybeHandle<Object> DebugEvaluate::Evaluate(
}

Handle<JSFunction> eval_fun;
ASSIGN_RETURN_ON_EXCEPTION(
isolate, eval_fun,
Compiler::GetFunctionFromEval(source, outer_info, context, SLOPPY,
NO_PARSE_RESTRICTION, 0),
Object);
ASSIGN_RETURN_ON_EXCEPTION(isolate, eval_fun,
Compiler::GetFunctionFromEval(
source, outer_info, context, SLOPPY,
NO_PARSE_RESTRICTION, RelocInfo::kNoPosition),
Object);

Handle<Object> result;
ASSIGN_RETURN_ON_EXCEPTION(
Expand Down
3 changes: 2 additions & 1 deletion src/debug/debug-frames.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ int FrameInspector::GetSourcePosition() {
return deoptimized_frame_->GetSourcePosition();
} else if (is_interpreted_) {
InterpretedFrame* frame = reinterpret_cast<InterpretedFrame*>(frame_);
BytecodeArray* bytecode_array = frame->GetBytecodeArray();
BytecodeArray* bytecode_array =
frame->function()->shared()->bytecode_array();
return bytecode_array->SourcePosition(frame->GetBytecodeOffset());
} else {
Code* code = frame_->LookupCode();
Expand Down
33 changes: 27 additions & 6 deletions src/debug/debug.cc
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,12 @@ BreakLocation BreakLocation::FromCodeOffset(Handle<DebugInfo> debug_info,
return it->GetBreakLocation();
}

FrameSummary GetFirstFrameSummary(JavaScriptFrame* frame) {
List<FrameSummary> frames(FLAG_max_inlining_levels + 1);
frame->Summarize(&frames);
return frames.first();
}

int CallOffsetFromCodeOffset(int code_offset, bool is_interpreted) {
// Code offset points to the instruction after the call. Subtract 1 to
// exclude that instruction from the search. For bytecode, the code offset
Expand All @@ -269,7 +275,7 @@ int CallOffsetFromCodeOffset(int code_offset, bool is_interpreted) {

BreakLocation BreakLocation::FromFrame(Handle<DebugInfo> debug_info,
JavaScriptFrame* frame) {
FrameSummary summary = FrameSummary::GetFirst(frame);
FrameSummary summary = GetFirstFrameSummary(frame);
int call_offset =
CallOffsetFromCodeOffset(summary.code_offset(), frame->is_interpreted());
return FromCodeOffset(debug_info, call_offset);
Expand Down Expand Up @@ -625,7 +631,7 @@ void Debug::Break(JavaScriptFrame* frame) {
step_break = location.IsTailCall();
// Fall through.
case StepIn: {
FrameSummary summary = FrameSummary::GetFirst(frame);
FrameSummary summary = GetFirstFrameSummary(frame);
int offset = summary.code_offset();
step_break = step_break || location.IsReturn() ||
(current_fp != last_fp) ||
Expand Down Expand Up @@ -1005,7 +1011,7 @@ void Debug::PrepareStep(StepAction step_action) {
}

// Get the debug info (create it if it does not exist).
FrameSummary summary = FrameSummary::GetFirst(frame);
FrameSummary summary = GetFirstFrameSummary(frame);
Handle<JSFunction> function(summary.function());
Handle<SharedFunctionInfo> shared(function->shared());
if (!EnsureDebugInfo(shared, function)) {
Expand All @@ -1016,7 +1022,7 @@ void Debug::PrepareStep(StepAction step_action) {
Handle<DebugInfo> debug_info(shared->GetDebugInfo());
// Refresh frame summary if the code has been recompiled for debugging.
if (AbstractCode::cast(shared->code()) != *summary.abstract_code()) {
summary = FrameSummary::GetFirst(frame);
summary = GetFirstFrameSummary(frame);
}

int call_offset =
Expand Down Expand Up @@ -1598,7 +1604,7 @@ bool Debug::IsBreakAtReturn(JavaScriptFrame* frame) {
if (!shared->HasDebugInfo()) return false;

DCHECK(!frame->is_optimized());
FrameSummary summary = FrameSummary::GetFirst(frame);
FrameSummary summary = GetFirstFrameSummary(frame);

Handle<DebugInfo> debug_info(shared->GetDebugInfo());
BreakLocation location =
Expand Down Expand Up @@ -1650,6 +1656,21 @@ Handle<FixedArray> Debug::GetLoadedScripts() {
}


void Debug::RecordEvalCaller(Handle<Script> script) {
script->set_compilation_type(Script::COMPILATION_TYPE_EVAL);
// For eval scripts add information on the function from which eval was
// called.
StackTraceFrameIterator it(script->GetIsolate());
if (!it.done()) {
script->set_eval_from_shared(it.frame()->function()->shared());
Code* code = it.frame()->LookupCode();
int offset = static_cast<int>(
it.frame()->pc() - code->instruction_start());
script->set_eval_from_instructions_offset(offset);
}
}


MaybeHandle<Object> Debug::MakeExecutionState() {
// Create the execution state object.
Handle<Object> argv[] = { isolate_->factory()->NewNumberFromInt(break_id()) };
Expand Down Expand Up @@ -2239,7 +2260,7 @@ void Debug::PrintBreakLocation() {
JavaScriptFrameIterator iterator(isolate_);
if (iterator.done()) return;
JavaScriptFrame* frame = iterator.frame();
FrameSummary summary = FrameSummary::GetFirst(frame);
FrameSummary summary = GetFirstFrameSummary(frame);
int source_position =
summary.abstract_code()->SourcePosition(summary.code_offset());
Handle<Object> script_obj(summary.function()->shared()->script(), isolate_);
Expand Down
3 changes: 3 additions & 0 deletions src/debug/debug.h
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,9 @@ class Debug {
static int ArchiveSpacePerThread();
void FreeThreadResources() { }

// Record function from which eval was called.
static void RecordEvalCaller(Handle<Script> script);

bool CheckExecutionState(int id) {
return is_active() && !debug_context().is_null() && break_id() != 0 &&
break_id() == id;
Expand Down
3 changes: 2 additions & 1 deletion src/debug/liveedit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1374,7 +1374,8 @@ static Handle<Script> CreateScriptCopy(Handle<Script> original) {
copy->set_type(original->type());
copy->set_context_data(original->context_data());
copy->set_eval_from_shared(original->eval_from_shared());
copy->set_eval_from_position(original->eval_from_position());
copy->set_eval_from_instructions_offset(
original->eval_from_instructions_offset());

// Copy all the flags, but clear compilation state.
copy->set_flags(original->flags());
Expand Down
2 changes: 1 addition & 1 deletion src/factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,7 @@ Handle<Script> Factory::NewScript(Handle<String> source) {
script->set_wrapper(heap->undefined_value());
script->set_line_ends(heap->undefined_value());
script->set_eval_from_shared(heap->undefined_value());
script->set_eval_from_position(0);
script->set_eval_from_instructions_offset(0);
script->set_shared_function_infos(Smi::FromInt(0));
script->set_flags(0);

Expand Down
12 changes: 3 additions & 9 deletions src/frames.cc
Original file line number Diff line number Diff line change
Expand Up @@ -977,12 +977,6 @@ FrameSummary::FrameSummary(Object* receiver, JSFunction* function,
CannotDeoptFromAsmCode(Code::cast(abstract_code), function));
}

FrameSummary FrameSummary::GetFirst(JavaScriptFrame* frame) {
List<FrameSummary> frames(FLAG_max_inlining_levels + 1);
frame->Summarize(&frames);
return frames.first();
}

void FrameSummary::Print() {
PrintF("receiver: ");
receiver_->ShortPrint();
Expand Down Expand Up @@ -1234,15 +1228,15 @@ void InterpretedFrame::PatchBytecodeOffset(int new_offset) {
SetExpression(index, Smi::FromInt(raw_offset));
}

BytecodeArray* InterpretedFrame::GetBytecodeArray() const {
Object* InterpretedFrame::GetBytecodeArray() const {
const int index = InterpreterFrameConstants::kBytecodeArrayExpressionIndex;
DCHECK_EQ(
InterpreterFrameConstants::kBytecodeArrayFromFp,
InterpreterFrameConstants::kExpressionsOffset - index * kPointerSize);
return BytecodeArray::cast(GetExpression(index));
return GetExpression(index);
}

void InterpretedFrame::PatchBytecodeArray(BytecodeArray* bytecode_array) {
void InterpretedFrame::PatchBytecodeArray(Object* bytecode_array) {
const int index = InterpreterFrameConstants::kBytecodeArrayExpressionIndex;
DCHECK_EQ(
InterpreterFrameConstants::kBytecodeArrayFromFp,
Expand Down
9 changes: 3 additions & 6 deletions src/frames.h
Original file line number Diff line number Diff line change
Expand Up @@ -640,16 +640,12 @@ class ExitFrame: public StackFrame {
friend class StackFrameIteratorBase;
};

class JavaScriptFrame;

class FrameSummary BASE_EMBEDDED {
public:
FrameSummary(Object* receiver, JSFunction* function,
AbstractCode* abstract_code, int code_offset,
bool is_constructor);

static FrameSummary GetFirst(JavaScriptFrame* frame);

Handle<Object> receiver() { return receiver_; }
Handle<JSFunction> function() { return function_; }
Handle<AbstractCode> abstract_code() { return abstract_code_; }
Expand Down Expand Up @@ -733,6 +729,7 @@ class StandardFrame : public StackFrame {
friend class SafeStackFrameIterator;
};


class JavaScriptFrame : public StandardFrame {
public:
Type type() const override { return JAVA_SCRIPT; }
Expand Down Expand Up @@ -900,11 +897,11 @@ class InterpretedFrame : public JavaScriptFrame {
void PatchBytecodeOffset(int new_offset);

// Returns the frame's current bytecode array.
BytecodeArray* GetBytecodeArray() const;
Object* GetBytecodeArray() const;

// Updates the frame's BytecodeArray with |bytecode_array|. Used by the
// debugger to swap execution onto a BytecodeArray patched with breakpoints.
void PatchBytecodeArray(BytecodeArray* bytecode_array);
void PatchBytecodeArray(Object* bytecode_array);

// Access to the interpreter register file for this frame.
Object* GetInterpreterRegister(int register_index) const;
Expand Down
12 changes: 6 additions & 6 deletions src/full-codegen/arm/full-codegen-arm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2561,8 +2561,8 @@ void FullCodeGenerator::EmitCall(Call* expr, ConvertReceiverMode mode) {
context()->DropAndPlug(1, r0);
}

void FullCodeGenerator::EmitResolvePossiblyDirectEval(Call* expr) {
int arg_count = expr->arguments()->length();

void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) {
// r4: copy of the first argument or undefined if it doesn't exist.
if (arg_count > 0) {
__ ldr(r4, MemOperand(sp, arg_count * kPointerSize));
Expand All @@ -2576,8 +2576,8 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(Call* expr) {
// r2: language mode.
__ mov(r2, Operand(Smi::FromInt(language_mode())));

// r1: the source position of the eval call.
__ mov(r1, Operand(Smi::FromInt(expr->position())));
// r1: the start position of the scope the calls resides in.
__ mov(r1, Operand(Smi::FromInt(scope()->start_position())));

// Do the runtime call.
__ Push(r4, r3, r2, r1);
Expand Down Expand Up @@ -2629,7 +2629,7 @@ void FullCodeGenerator::PushCalleeAndWithBaseObject(Call* expr) {

void FullCodeGenerator::EmitPossiblyEvalCall(Call* expr) {
// In a call to eval, we first call
// Runtime_ResolvePossiblyDirectEval to resolve the function we need
// RuntimeHidden_asResolvePossiblyDirectEval to resolve the function we need
// to call. Then we call the resolved function using the given arguments.
ZoneList<Expression*>* args = expr->arguments();
int arg_count = args->length();
Expand All @@ -2645,7 +2645,7 @@ void FullCodeGenerator::EmitPossiblyEvalCall(Call* expr) {
// resolve eval.
__ ldr(r1, MemOperand(sp, (arg_count + 1) * kPointerSize));
__ push(r1);
EmitResolvePossiblyDirectEval(expr);
EmitResolvePossiblyDirectEval(arg_count);

// Touch up the stack with the resolved function.
__ str(r0, MemOperand(sp, (arg_count + 1) * kPointerSize));
Expand Down
Loading

0 comments on commit 5af0a68

Please sign in to comment.