Skip to content

Commit

Permalink
Bug 1406957 part 2 - Rewrite this-creation to be simpler and more con…
Browse files Browse the repository at this point in the history 
…sistent. r=tcampbell
  • Loading branch information
@jandem
jandem committed Oct 11, 2017
1 parent 53cef4d commit 3c7f57d
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 76 deletions.
9 changes: 0 additions & 9 deletions js/src/jit/BaselineJIT.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,19 +368,10 @@ jit::CanEnterBaselineMethod(JSContext* cx, RunState& state)
{
if (state.isInvoke()) {
InvokeState& invoke = *state.asInvoke();

if (invoke.args().length() > BASELINE_MAX_ARGS_LENGTH) {
JitSpew(JitSpew_BaselineAbort, "Too many arguments (%u)", invoke.args().length());
return Method_CantCompile;
}

if (!state.maybeCreateThisForConstructor(cx)) {
if (cx->isThrowingOutOfMemory()) {
cx->recoverFromOutOfMemory();
return Method_Skipped;
}
return Method_Error;
}
} else {
if (state.asExecute()->isDebuggerEval()) {
JitSpew(JitSpew_BaselineAbort, "debugger frame");
Expand Down
15 changes: 2 additions & 13 deletions js/src/jit/Ion.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2522,14 +2522,6 @@ jit::CanEnter(JSContext* cx, RunState& state)
ForbidCompilation(cx, script);
return Method_CantCompile;
}

if (!state.maybeCreateThisForConstructor(cx)) {
if (cx->isThrowingOutOfMemory()) {
cx->recoverFromOutOfMemory();
return Method_Skipped;
}
return Method_Error;
}
}

// If --ion-eager is used, compile with Baseline first, so that we
Expand All @@ -2540,11 +2532,8 @@ jit::CanEnter(JSContext* cx, RunState& state)
return status;
}

// Skip if the script is being compiled off thread or can't be
// Ion-compiled (again). MaybeCreateThisForConstructor could have
// started an Ion compilation or marked the script as uncompilable.
if (script->isIonCompilingOffThread() || !script->canIonCompile())
return Method_Skipped;
MOZ_ASSERT(!script->isIonCompilingOffThread());
MOZ_ASSERT(script->canIonCompile());

// Attempt compilation. Returns Method_Compiled if already compiled.
MethodStatus status = Compile(cx, script, nullptr, nullptr);
Expand Down
67 changes: 40 additions & 27 deletions js/src/vm/Interpreter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,29 +334,35 @@ js::ValueToCallable(JSContext* cx, HandleValue v, int numToSkip, MaybeConstruct
return nullptr;
}

bool
RunState::maybeCreateThisForConstructor(JSContext* cx)
{
if (isInvoke()) {
InvokeState& invoke = *asInvoke();
if (invoke.constructing() && invoke.args().thisv().isPrimitive()) {
RootedObject callee(cx, &invoke.args().callee());
if (callee->isBoundFunction()) {
invoke.args().setThis(MagicValue(JS_UNINITIALIZED_LEXICAL));
} else if (script()->isDerivedClassConstructor()) {
MOZ_ASSERT(callee->as<JSFunction>().isClassConstructor());
invoke.args().setThis(MagicValue(JS_UNINITIALIZED_LEXICAL));
} else {
MOZ_ASSERT(invoke.args().thisv().isMagic(JS_IS_CONSTRUCTING));
RootedObject newTarget(cx, &invoke.args().newTarget().toObject());
NewObjectKind newKind = invoke.createSingleton() ? SingletonObject : GenericObject;
JSObject* obj = CreateThisForFunction(cx, callee, newTarget, newKind);
if (!obj)
return false;
invoke.args().setThis(ObjectValue(*obj));
}
}
static bool
MaybeCreateThisForConstructor(JSContext* cx, JSScript* calleeScript, const CallArgs& args,
bool createSingleton)
{
if (args.thisv().isObject())
return true;

RootedObject callee(cx, &args.callee());
if (callee->isBoundFunction()) {
args.setThis(MagicValue(JS_UNINITIALIZED_LEXICAL));
return true;
}

if (calleeScript->isDerivedClassConstructor()) {
MOZ_ASSERT(callee->as<JSFunction>().isClassConstructor());
args.setThis(MagicValue(JS_UNINITIALIZED_LEXICAL));
return true;
}

MOZ_ASSERT(args.thisv().isMagic(JS_IS_CONSTRUCTING));

RootedObject newTarget(cx, &args.newTarget().toObject());
NewObjectKind newKind = createSingleton ? SingletonObject : GenericObject;

JSObject* obj = CreateThisForFunction(cx, callee, newTarget, newKind);
if (!obj)
return false;

args.setThis(ObjectValue(*obj));
return true;
}

Expand Down Expand Up @@ -502,11 +508,15 @@ js::InternalCallOrConstruct(JSContext* cx, const CallArgs& args, MaybeConstruct

// Check to see if createSingleton flag should be set for this frame.
if (construct) {
bool createSingleton = false;
jsbytecode* pc;
if (JSScript* script = cx->currentScript(&pc)) {
if (ObjectGroup::useSingletonForNewObject(cx, script, pc))
state.setCreateSingleton();
createSingleton = true;
}

if (!MaybeCreateThisForConstructor(cx, state.script(), args, createSingleton))
return false;
}

bool ok = RunScript(cx, state);
Expand Down Expand Up @@ -3097,16 +3107,19 @@ CASE(JSOP_FUNCALL)
if (!funScript)
goto error;

bool createSingleton = ObjectGroup::useSingletonForNewObject(cx, script, REGS.pc);
bool createSingleton = false;
if (construct) {
createSingleton = ObjectGroup::useSingletonForNewObject(cx, script, REGS.pc);

if (!MaybeCreateThisForConstructor(cx, funScript, args, createSingleton))
goto error;
}

TypeMonitorCall(cx, args, construct);

{
InvokeState state(cx, args, construct);

if (createSingleton)
state.setCreateSingleton();

if (!createSingleton && jit::IsIonEnabled(cx)) {
jit::MethodStatus status = jit::CanEnter(cx, state);
if (status == jit::Method_Error)
Expand Down
9 changes: 1 addition & 8 deletions js/src/vm/Interpreter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,6 @@ class RunState
InterpreterFrame* pushInterpreterFrame(JSContext* cx);
inline void setReturnValue(const Value& v);

bool maybeCreateThisForConstructor(JSContext* cx);

private:
RunState(const RunState& other) = delete;
RunState(const ExecuteState& other) = delete;
Expand Down Expand Up @@ -281,19 +279,14 @@ class InvokeState final : public RunState
{
const CallArgs& args_;
MaybeConstruct construct_;
bool createSingleton_;

public:
InvokeState(JSContext* cx, const CallArgs& args, MaybeConstruct construct)
: RunState(cx, Invoke, args.callee().as<JSFunction>().nonLazyScript()),
args_(args),
construct_(construct),
createSingleton_(false)
construct_(construct)
{ }

bool createSingleton() const { return createSingleton_; }
void setCreateSingleton() { createSingleton_ = true; }

bool constructing() const { return construct_; }
const CallArgs& args() const { return args_; }

Expand Down
21 changes: 2 additions & 19 deletions js/src/vm/Stack.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,25 +244,8 @@ InterpreterFrame::prologue(JSContext* cx)
if (callee().needsFunctionEnvironmentObjects() && !initFunctionEnvironmentObjects(cx))
return false;

if (isConstructing()) {
if (callee().isBoundFunction()) {
thisArgument() = MagicValue(JS_UNINITIALIZED_LEXICAL);
} else if (script->isDerivedClassConstructor()) {
MOZ_ASSERT(callee().isClassConstructor());
thisArgument() = MagicValue(JS_UNINITIALIZED_LEXICAL);
} else if (thisArgument().isObject()) {
// Nothing to do. Correctly set.
} else {
MOZ_ASSERT(thisArgument().isMagic(JS_IS_CONSTRUCTING));
RootedObject callee(cx, &this->callee());
RootedObject newTarget(cx, &this->newTarget().toObject());
JSObject* obj = CreateThisForFunction(cx, callee, newTarget,
createSingleton() ? SingletonObject : GenericObject);
if (!obj)
return false;
thisArgument() = ObjectValue(*obj);
}
}
MOZ_ASSERT_IF(isConstructing(),
thisArgument().isObject() || thisArgument().isMagic(JS_UNINITIALIZED_LEXICAL));

return probes::EnterScript(cx, script, script->functionNonDelazifying(), this);
}
Expand Down

0 comments on commit 3c7f57d

Please sign in to comment.