Bug 1035287 - Part 2: Refactor js::SavedStacks::insertFrames to use iteration instead of recursion. r=shu

This commit is contained in:
Nick Fitzgerald 2014-07-23 09:15:00 +02:00
parent 3a832415cc
commit f2be1a7e7c
3 changed files with 205 additions and 107 deletions

View File

@ -0,0 +1,7 @@
// |jit-test| exitstatus: 3
enableTrackAllocations();
function f() {
eval('f();');
}
f();

View File

@ -13,6 +13,7 @@
#include "jsnum.h"
#include "gc/Marking.h"
#include "js/Vector.h"
#include "vm/GlobalObject.h"
#include "vm/StringBuffer.h"
@ -25,7 +26,7 @@ using mozilla::HashString;
namespace js {
struct SavedFrame::Lookup {
Lookup(JSAtom *source, size_t line, size_t column, JSAtom *functionDisplayName,
Lookup(JSAtom *source, uint32_t line, uint32_t column, JSAtom *functionDisplayName,
SavedFrame *parent, JSPrincipals *principals)
: source(source),
line(line),
@ -38,8 +39,8 @@ struct SavedFrame::Lookup {
}
JSAtom *source;
size_t line;
size_t column;
uint32_t line;
uint32_t column;
JSAtom *functionDisplayName;
SavedFrame *parent;
JSPrincipals *principals;
@ -48,12 +49,13 @@ struct SavedFrame::Lookup {
class SavedFrame::AutoLookupRooter : public JS::CustomAutoRooter
{
public:
AutoLookupRooter(JSContext *cx, JSAtom *source, size_t line, size_t column,
AutoLookupRooter(JSContext *cx, JSAtom *source, uint32_t line, uint32_t column,
JSAtom *functionDisplayName, SavedFrame *parent, JSPrincipals *principals)
: JS::CustomAutoRooter(cx),
value(source, line, column, functionDisplayName, parent, principals) {}
operator const SavedFrame::Lookup&() const { return value; }
SavedFrame::Lookup &get() { return value; }
private:
virtual void trace(JSTracer *trc) {
@ -69,6 +71,16 @@ class SavedFrame::AutoLookupRooter : public JS::CustomAutoRooter
SavedFrame::Lookup value;
};
class SavedFrame::HandleLookup
{
public:
HandleLookup(SavedFrame::AutoLookupRooter &lookup) : ref(lookup) { }
SavedFrame::Lookup *operator->() { return &ref.get(); }
operator const SavedFrame::Lookup&() const { return ref; }
private:
SavedFrame::AutoLookupRooter &ref;
};
/* static */ HashNumber
SavedFrame::HashPolicy::hash(const Lookup &lookup)
{
@ -150,14 +162,14 @@ SavedFrame::getSource()
return &s->asAtom();
}
size_t
uint32_t
SavedFrame::getLine()
{
const Value &v = getReservedSlot(JSSLOT_LINE);
return v.toInt32();
}
size_t
uint32_t
SavedFrame::getColumn()
{
const Value &v = getReservedSlot(JSSLOT_COLUMN);
@ -191,25 +203,25 @@ SavedFrame::getPrincipals()
}
void
SavedFrame::initFromLookup(const Lookup &lookup)
SavedFrame::initFromLookup(SavedFrame::HandleLookup lookup)
{
JS_ASSERT(lookup.source);
JS_ASSERT(lookup->source);
JS_ASSERT(getReservedSlot(JSSLOT_SOURCE).isUndefined());
setReservedSlot(JSSLOT_SOURCE, StringValue(lookup.source));
setReservedSlot(JSSLOT_SOURCE, StringValue(lookup->source));
setReservedSlot(JSSLOT_LINE, NumberValue(lookup.line));
setReservedSlot(JSSLOT_COLUMN, NumberValue(lookup.column));
setReservedSlot(JSSLOT_LINE, NumberValue(lookup->line));
setReservedSlot(JSSLOT_COLUMN, NumberValue(lookup->column));
setReservedSlot(JSSLOT_FUNCTIONDISPLAYNAME,
lookup.functionDisplayName
? StringValue(lookup.functionDisplayName)
lookup->functionDisplayName
? StringValue(lookup->functionDisplayName)
: NullValue());
setReservedSlot(JSSLOT_PARENT, ObjectOrNullValue(lookup.parent));
setReservedSlot(JSSLOT_PRIVATE_PARENT, PrivateValue(lookup.parent));
setReservedSlot(JSSLOT_PARENT, ObjectOrNullValue(lookup->parent));
setReservedSlot(JSSLOT_PRIVATE_PARENT, PrivateValue(lookup->parent));
JS_ASSERT(getReservedSlot(JSSLOT_PRINCIPALS).isUndefined());
if (lookup.principals)
JS_HoldPrincipals(lookup.principals);
setReservedSlot(JSSLOT_PRINCIPALS, PrivateValue(lookup.principals));
if (lookup->principals)
JS_HoldPrincipals(lookup->principals);
setReservedSlot(JSSLOT_PRINCIPALS, PrivateValue(lookup->principals));
}
bool
@ -480,78 +492,74 @@ bool
SavedStacks::insertFrames(JSContext *cx, FrameIter &iter, MutableHandleSavedFrame frame,
unsigned maxFrameCount)
{
if (iter.done()) {
frame.set(nullptr);
// In order to lookup a cached SavedFrame object, we need to have its parent
// SavedFrame, which means we need to walk the stack from oldest frame to
// youngest. However, FrameIter walks the stack from youngest frame to
// oldest. The solution is to append stack frames to a vector as we walk the
// stack with FrameIter, and then do a second pass through that vector in
// reverse order after the traversal has completed and get or create the
// SavedFrame objects at that time.
//
// To avoid making many copies of FrameIter (whose copy constructor is
// relatively slow), we save the subset of FrameIter's data that is relevant
// to our needs in a FrameState object, and maintain a vector of FrameState
// objects instead of a vector of FrameIter objects.
// Accumulate the vector of FrameState objects in |stackState|.
AutoFrameStateVector stackState(cx);
while (!iter.done()) {
AutoLocationValueRooter location(cx);
{
AutoCompartment ac(cx, iter.compartment());
if (!cx->compartment()->savedStacks().getLocation(cx, iter, &location))
return false;
}
{
FrameState frameState(iter);
frameState.location = location.get();
if (!stackState->append(frameState))
return false;
}
++iter;
if (maxFrameCount == 0) {
// If maxFrameCount is zero, then there's no limit on the number of
// frames.
continue;
} else if (maxFrameCount == 1) {
// Since we were only asked to save one frame, do not continue
// walking the stack and saving frame state.
break;
} else {
maxFrameCount--;
}
}
// Iterate through |stackState| in reverse order and get or create the
// actual SavedFrame instances.
RootedSavedFrame parentFrame(cx, nullptr);
for (size_t i = stackState->length(); i != 0; i--) {
SavedFrame::AutoLookupRooter lookup(cx,
stackState[i-1].location.source,
stackState[i-1].location.line,
stackState[i-1].location.column,
stackState[i-1].name,
parentFrame,
stackState[i-1].principals);
parentFrame.set(getOrCreateSavedFrame(cx, lookup));
if (!parentFrame)
return false;
}
frame.set(parentFrame);
return true;
}
// Don't report the over-recursion error because if we are blowing the stack
// here, we already blew the stack in JS, reported it, and we are creating
// the saved stack for the over-recursion error object. We do this check
// here, rather than inside saveCurrentStack, because in some cases we will
// pass the check there, despite later failing the check here (for example,
// in js/src/jit-test/tests/saved-stacks/bug-1006876-too-much-recursion.js).
JS_CHECK_RECURSION_DONT_REPORT(cx, return false);
JSPrincipals* principals = iter.compartment()->principals;
RootedAtom name(cx, iter.isNonEvalFunctionFrame() ? iter.functionDisplayAtom() : nullptr);
// When we have a |JSScript| for this frame, use |getLocation| to get a
// potentially memoized location result and copy it into |location|. When we
// do not have a |JSScript| for this frame (asm.js frames), we take a slow
// path that doesn't employ memoization, and update |location|'s slots
// directly.
AutoLocationValueRooter location(cx);
if (iter.hasScript()) {
JSScript *script = iter.script();
jsbytecode *pc = iter.pc();
{
AutoCompartment ac(cx, iter.compartment());
if (!cx->compartment()->savedStacks().getLocation(cx, script, pc, &location))
return false;
}
} else {
const char *filename = iter.scriptFilename();
if (!filename)
filename = "";
location.get().source = Atomize(cx, filename, strlen(filename));
if (!location.get().source)
return false;
uint32_t column;
location.get().line = iter.computeLine(&column);
location.get().column = column;
}
RootedSavedFrame parentFrame(cx);
// If maxFrameCount is zero, then there's no limit on the number of frames.
if (maxFrameCount == 0) {
if (!insertFrames(cx, ++iter, &parentFrame, 0))
return false;
} else if (maxFrameCount == 1) {
// Since we were only asked to save one frame, the SavedFrame we're
// building here should have no parent, even if there are older frames
// on the stack.
parentFrame = nullptr;
} else {
if (!insertFrames(cx, ++iter, &parentFrame, maxFrameCount - 1))
return false;
}
SavedFrame::AutoLookupRooter lookup(cx,
location.get().source,
location.get().line,
location.get().column,
name,
parentFrame,
principals);
frame.set(getOrCreateSavedFrame(cx, lookup));
return frame.get() != nullptr;
}
SavedFrame *
SavedStacks::getOrCreateSavedFrame(JSContext *cx, const SavedFrame::Lookup &lookup)
SavedStacks::getOrCreateSavedFrame(JSContext *cx, SavedFrame::HandleLookup lookup)
{
SavedFrame::Set::AddPtr p = frames.lookupForAdd(lookup);
if (p)
@ -594,7 +602,7 @@ SavedStacks::getOrCreateSavedFramePrototype(JSContext *cx)
}
SavedFrame *
SavedStacks::createFrameFromLookup(JSContext *cx, const SavedFrame::Lookup &lookup)
SavedStacks::createFrameFromLookup(JSContext *cx, SavedFrame::HandleLookup lookup)
{
RootedObject proto(cx, getOrCreateSavedFramePrototype(cx));
if (!proto)
@ -640,14 +648,33 @@ SavedStacks::sweepPCLocationMap()
}
bool
SavedStacks::getLocation(JSContext *cx, JSScript *script, jsbytecode *pc,
MutableHandleLocationValue locationp)
SavedStacks::getLocation(JSContext *cx, const FrameIter &iter, MutableHandleLocationValue locationp)
{
// We should only ever be caching location values for scripts in this
// compartment. Otherwise, we would get dead cross-compartment scripts in
// the cache because our compartment's sweep method isn't called when their
// compartment gets collected.
assertSameCompartment(cx, this, script);
assertSameCompartment(cx, this, iter.compartment());
// When we have a |JSScript| for this frame, use a potentially memoized
// location from our PCLocationMap and copy it into |locationp|. When we do
// not have a |JSScript| for this frame (asm.js frames), we take a slow path
// that doesn't employ memoization, and update |locationp|'s slots directly.
if (!iter.hasScript()) {
const char *filename = iter.scriptFilename();
if (!filename)
filename = "";
locationp->source = Atomize(cx, filename, strlen(filename));
if (!locationp->source)
return false;
locationp->line = iter.computeLine(&locationp->column);
return true;
}
RootedScript script(cx, iter.script());
jsbytecode *pc = iter.pc();
PCKey key(script, pc);
PCLocationMap::AddPtr p = pcLocationMap.lookupForAdd(key);
@ -670,6 +697,36 @@ SavedStacks::getLocation(JSContext *cx, JSScript *script, jsbytecode *pc,
return true;
}
SavedStacks::FrameState::FrameState(const FrameIter &iter)
: principals(iter.compartment()->principals),
name(iter.isNonEvalFunctionFrame() ? iter.functionDisplayAtom() : nullptr),
location()
{
if (principals)
JS_HoldPrincipals(principals);
}
SavedStacks::FrameState::FrameState(const FrameState &fs)
: principals(fs.principals),
name(fs.name),
location(fs.location)
{
if (principals)
JS_HoldPrincipals(principals);
}
SavedStacks::FrameState::~FrameState() {
if (principals)
JS_DropPrincipals(TlsPerThreadData.get()->runtimeFromMainThread(), principals);
}
void
SavedStacks::FrameState::trace(JSTracer *trc) {
if (name)
gc::MarkStringUnbarriered(trc, &name, "SavedStacks::FrameState::name");
location.trace(trc);
}
bool
SavedStacksMetadataCallback(JSContext *cx, JSObject **pmetadata)
{

View File

@ -33,8 +33,8 @@ class SavedFrame : public JSObject {
// Convenient getters for SavedFrame's reserved slots for use from C++.
JSAtom *getSource();
size_t getLine();
size_t getColumn();
uint32_t getLine();
uint32_t getColumn();
JSAtom *getFunctionDisplayName();
SavedFrame *getParent();
JSPrincipals *getPrincipals();
@ -49,9 +49,10 @@ class SavedFrame : public JSObject {
SystemAllocPolicy> Set;
class AutoLookupRooter;
class HandleLookup;
private:
void initFromLookup(const Lookup &lookup);
void initFromLookup(HandleLookup lookup);
enum {
// The reserved slots in the SavedFrame class.
@ -118,11 +119,11 @@ class SavedStacks {
bool insertFrames(JSContext *cx, FrameIter &iter, MutableHandleSavedFrame frame,
unsigned maxFrameCount = 0);
SavedFrame *getOrCreateSavedFrame(JSContext *cx, const SavedFrame::Lookup &lookup);
SavedFrame *getOrCreateSavedFrame(JSContext *cx, SavedFrame::HandleLookup lookup);
// |SavedFrame.prototype| is created lazily and held weakly. It should only
// be accessed through this method.
JSObject *getOrCreateSavedFramePrototype(JSContext *cx);
SavedFrame *createFrameFromLookup(JSContext *cx, const SavedFrame::Lookup &lookup);
SavedFrame *createFrameFromLookup(JSContext *cx, SavedFrame::HandleLookup lookup);
// Cache for memoizing PCToLineNumber lookups.
@ -135,15 +136,20 @@ class SavedStacks {
struct LocationValue {
LocationValue() : source(nullptr), line(0), column(0) { }
LocationValue(JSAtom *source, size_t line, size_t column)
LocationValue(JSAtom *source, size_t line, uint32_t column)
: source(source),
line(line),
column(column)
{ }
void trace(JSTracer *trc) {
if (source)
gc::MarkString(trc, &source, "SavedStacks::LocationValue::source");
}
PreBarrieredAtom source;
size_t line;
size_t column;
uint32_t column;
};
class MOZ_STACK_CLASS AutoLocationValueRooter : public JS::CustomAutoRooter
@ -153,18 +159,13 @@ class SavedStacks {
: JS::CustomAutoRooter(cx),
value() {}
void set(LocationValue &loc) {
value = loc;
}
LocationValue &get() {
return value;
}
inline LocationValue *operator->() { return &value; }
void set(LocationValue &loc) { value = loc; }
LocationValue &get() { return value; }
private:
virtual void trace(JSTracer *trc) {
if (value.source)
gc::MarkString(trc, &value.source, "SavedStacks::LocationValue::source");
value.trace(trc);
}
SavedStacks::LocationValue value;
@ -176,9 +177,8 @@ class SavedStacks {
inline MOZ_IMPLICIT MutableHandleLocationValue(AutoLocationValueRooter *location)
: location(location) {}
void set(LocationValue &loc) {
location->set(loc);
}
inline LocationValue *operator->() { return &location->get(); }
void set(LocationValue &loc) { location->set(loc); }
private:
AutoLocationValueRooter *location;
@ -203,8 +203,42 @@ class SavedStacks {
PCLocationMap pcLocationMap;
void sweepPCLocationMap();
bool getLocation(JSContext *cx, JSScript *script, jsbytecode *pc,
MutableHandleLocationValue locationp);
bool getLocation(JSContext *cx, const FrameIter &iter, MutableHandleLocationValue locationp);
struct FrameState
{
FrameState() : principals(nullptr), name(nullptr), location() { }
FrameState(const FrameIter &iter);
FrameState(const FrameState &fs);
~FrameState();
void trace(JSTracer *trc);
JSPrincipals *principals;
JSAtom *name;
LocationValue location;
};
class MOZ_STACK_CLASS AutoFrameStateVector : public JS::CustomAutoRooter {
public:
AutoFrameStateVector(JSContext *cx)
: JS::CustomAutoRooter(cx),
frames(cx)
{ }
typedef Vector<FrameState> FrameStateVector;
inline FrameStateVector *operator->() { return &frames; }
inline FrameState &operator[](size_t i) { return frames[i]; }
private:
FrameStateVector frames;
virtual void trace(JSTracer *trc) {
for (size_t i = 0; i < frames.length(); i++)
frames[i].trace(trc);
}
};
};
bool SavedStacksMetadataCallback(JSContext *cx, JSObject **pmetadata);