Skip to content

Commit

Permalink
Bug 1535154 - Merge PrivateScriptData scopes/objects/bigints arrays i…
Browse files Browse the repository at this point in the history
…nto a single array of GC things. r=tcampbell,jonco

Once the other data is moved out of PrivateScriptData, this GC-thing array will be stored
at a fixed offset. At that point we can simplify PrivateScriptData and get fast indexing
into this array, important for the Baseline Interpreter.

Differential Revision: https://phabricator.services.mozilla.com/D34902

--HG--
extra : moz-landing-system : lando
  • Loading branch information
jandem committed Jun 20, 2019
1 parent 9c57855 commit 7542b54
Show file tree
Hide file tree
Showing 18 changed files with 462 additions and 496 deletions.
2 changes: 2 additions & 0 deletions js/public/HeapAPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ struct Symbol {
*/
class JS_FRIEND_API GCCellPtr {
public:
GCCellPtr() : GCCellPtr(nullptr) {}

// Construction from a void* and trace kind.
GCCellPtr(void* gcthing, JS::TraceKind traceKind)
: ptr(checkedCast(gcthing, traceKind)) {}
Expand Down
17 changes: 15 additions & 2 deletions js/src/builtin/Eval.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,23 @@ static void AssertInnerizedEnvironmentChain(JSContext* cx, JSObject& env) {
}

static bool IsEvalCacheCandidate(JSScript* script) {
if (!script->isDirectEvalInFunction()) {
return false;
}

// Make sure there are no inner objects which might use the wrong parent
// and/or call scope by reusing the previous eval's script.
return script->isDirectEvalInFunction() && !script->hasSingletons() &&
!script->hasObjects();
if (script->hasSingletons()) {
return false;
}

for (JS::GCCellPtr gcThing : script->gcthings()) {
if (gcThing.is<JSObject>()) {
return false;
}
}

return true;
}

/* static */
Expand Down
51 changes: 34 additions & 17 deletions js/src/frontend/BytecodeEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -923,25 +923,34 @@ bool BytecodeEmitter::emitAtomOp(uint32_t atomIndex, JSOp op) {

bool BytecodeEmitter::emitInternedScopeOp(uint32_t index, JSOp op) {
MOZ_ASSERT(JOF_OPTYPE(op) == JOF_SCOPE);
MOZ_ASSERT(index < perScriptData().scopeList().length());
MOZ_ASSERT(index < perScriptData().gcThingList().length());
return emitIndex32(op, index);
}

bool BytecodeEmitter::emitInternedObjectOp(uint32_t index, JSOp op) {
MOZ_ASSERT(JOF_OPTYPE(op) == JOF_OBJECT);
MOZ_ASSERT(index < perScriptData().objectList().length);
MOZ_ASSERT(index < perScriptData().gcThingList().length());
return emitIndex32(op, index);
}

bool BytecodeEmitter::emitObjectOp(ObjectBox* objbox, JSOp op) {
return emitInternedObjectOp(perScriptData().objectList().add(objbox), op);
uint32_t index;
if (!perScriptData().gcThingList().append(objbox, &index)) {
return false;
}

return emitInternedObjectOp(index, op);
}

bool BytecodeEmitter::emitObjectPairOp(ObjectBox* objbox1, ObjectBox* objbox2,
JSOp op) {
uint32_t index = perScriptData().objectList().add(objbox1);
perScriptData().objectList().add(objbox2);
return emitInternedObjectOp(index, op);
uint32_t index1, index2;
if (!perScriptData().gcThingList().append(objbox1, &index1) ||
!perScriptData().gcThingList().append(objbox2, &index2)) {
return false;
}

return emitInternedObjectOp(index1, op);
}

bool BytecodeEmitter::emitRegExp(uint32_t index) {
Expand Down Expand Up @@ -1678,7 +1687,7 @@ bool BytecodeEmitter::emitNewInit() {
return true;
}

bool BytecodeEmitter::iteratorResultShape(unsigned* shape) {
bool BytecodeEmitter::iteratorResultShape(uint32_t* shape) {
// No need to do any guessing for the object kind, since we know exactly how
// many properties we plan to have.
gc::AllocKind kind = gc::GetGCObjectKind(2);
Expand All @@ -1704,13 +1713,11 @@ bool BytecodeEmitter::iteratorResultShape(unsigned* shape) {
return false;
}

*shape = perScriptData().objectList().add(objbox);

return true;
return perScriptData().gcThingList().append(objbox, shape);
}

bool BytecodeEmitter::emitPrepareIteratorResult() {
unsigned shape;
uint32_t shape;
if (!iteratorResultShape(&shape)) {
return false;
}
Expand Down Expand Up @@ -5006,10 +5013,11 @@ bool BytecodeEmitter::emitCopyDataProperties(CopyOption option) {
}

bool BytecodeEmitter::emitBigIntOp(BigInt* bigint) {
if (!perScriptData().bigIntList().append(bigint)) {
uint32_t index;
if (!perScriptData().gcThingList().append(bigint, &index)) {
return false;
}
return emitIndex32(JSOP_BIGINT, perScriptData().bigIntList().length() - 1);
return emitIndex32(JSOP_BIGINT, index);
}

bool BytecodeEmitter::emitIterator() {
Expand Down Expand Up @@ -8246,7 +8254,11 @@ bool BytecodeEmitter::replaceNewInitWithNewObject(JSObject* obj,
JSOP_NEWINIT_LENGTH == JSOP_NEWOBJECT_LENGTH,
"newinit and newobject must have equal length to edit in-place");

uint32_t index = perScriptData().objectList().add(objbox);
uint32_t index;
if (!perScriptData().gcThingList().append(objbox, &index)) {
return false;
}

jsbytecode* code = bytecodeSection().code(offset);

MOZ_ASSERT(code[0] == JSOP_NEWINIT);
Expand Down Expand Up @@ -9339,12 +9351,17 @@ bool BytecodeEmitter::emitTree(
}
break;

case ParseNodeKind::RegExpExpr:
if (!emitRegExp(perScriptData().objectList().add(
pn->as<RegExpLiteral>().objbox()))) {
case ParseNodeKind::RegExpExpr: {
ObjectBox* obj = pn->as<RegExpLiteral>().objbox();
uint32_t index;
if (!perScriptData().gcThingList().append(obj, &index)) {
return false;
}
if (!emitRegExp(index)) {
return false;
}
break;
}

case ParseNodeKind::TrueExpr:
if (!emit1(JSOP_TRUE)) {
Expand Down
7 changes: 3 additions & 4 deletions js/src/frontend/BytecodeEmitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,11 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
}

Scope* outermostScope() const {
return perScriptData().scopeList().vector[0];
return perScriptData().gcThingList().firstScope();
}
Scope* innermostScope() const;
Scope* bodyScope() const {
MOZ_ASSERT(bodyScopeIndex < perScriptData().scopeList().length());
return perScriptData().scopeList().vector[bodyScopeIndex];
return perScriptData().gcThingList().getScope(bodyScopeIndex);
}

MOZ_ALWAYS_INLINE
Expand Down Expand Up @@ -546,7 +545,7 @@ struct MOZ_STACK_CLASS BytecodeEmitter {

MOZ_MUST_USE bool emitPrepareIteratorResult();
MOZ_MUST_USE bool emitFinishIteratorResult(bool done);
MOZ_MUST_USE bool iteratorResultShape(unsigned* shape);
MOZ_MUST_USE bool iteratorResultShape(uint32_t* shape);

MOZ_MUST_USE bool emitGetDotGeneratorInInnermostScope() {
return emitGetDotGeneratorInScope(*innermostEmitterScope());
Expand Down
44 changes: 9 additions & 35 deletions js/src/frontend/BytecodeSection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,44 +18,20 @@
using namespace js;
using namespace js::frontend;

void CGBigIntList::finish(mozilla::Span<GCPtrBigInt> array) {
MOZ_ASSERT(length() == array.size());

for (unsigned i = 0; i < length(); i++) {
array[i].init(vector[i]);
}
}
bool GCThingList::append(ObjectBox* objbox, uint32_t* index) {
// Append the object to the vector and return the index in *index. Also add
// the ObjectBox to the |lastbox| linked list for finishInnerFunctions below.

/*
* Find the index of the given object for code generator.
*
* Since the emitter refers to each parsed object only once, for the index we
* use the number of already indexed objects. We also add the object to a list
* to convert the list to a fixed-size array when we complete code generation,
* see js::CGObjectList::finish below.
*/
unsigned CGObjectList::add(ObjectBox* objbox) {
MOZ_ASSERT(objbox->isObjectBox());
MOZ_ASSERT(!objbox->emitLink);
objbox->emitLink = lastbox;
lastbox = objbox;
return length++;
}

void CGObjectList::finish(mozilla::Span<GCPtrObject> array) {
MOZ_ASSERT(length <= INDEX_LIMIT);
MOZ_ASSERT(length == array.size());

ObjectBox* objbox = lastbox;
for (GCPtrObject& obj : mozilla::Reversed(array)) {
MOZ_ASSERT(obj == nullptr);
MOZ_ASSERT(objbox->object()->isTenured());
obj.init(objbox->object());
objbox = objbox->emitLink;
}
*index = vector.length();
return vector.append(JS::GCCellPtr(objbox->object()));
}

void CGObjectList::finishInnerFunctions() {
void GCThingList::finishInnerFunctions() {
ObjectBox* objbox = lastbox;
while (objbox) {
if (objbox->isFunctionBox()) {
Expand All @@ -65,12 +41,12 @@ void CGObjectList::finishInnerFunctions() {
}
}

void CGScopeList::finish(mozilla::Span<GCPtrScope> array) {
void GCThingList::finish(mozilla::Span<JS::GCCellPtr> array) {
MOZ_ASSERT(length() <= INDEX_LIMIT);
MOZ_ASSERT(length() == array.size());

for (uint32_t i = 0; i < length(); i++) {
array[i].init(vector[i]);
array[i] = vector[i].get().get();
}
}

Expand Down Expand Up @@ -170,8 +146,6 @@ void BytecodeSection::updateDepth(BytecodeOffset target) {
}

PerScriptData::PerScriptData(JSContext* cx)
: scopeList_(cx),
bigIntList_(cx),
atomIndices_(cx->frontendCollectionPool()) {}
: gcThingList_(cx), atomIndices_(cx->frontendCollectionPool()) {}

bool PerScriptData::init(JSContext* cx) { return atomIndices_.acquire(cx); }
85 changes: 37 additions & 48 deletions js/src/frontend/BytecodeSection.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#define frontend_BytecodeSection_h

#include "mozilla/Attributes.h" // MOZ_MUST_USE, MOZ_STACK_CLASS
#include "mozilla/Maybe.h" // mozilla::Maybe
#include "mozilla/Span.h" // mozilla::Span

#include <stddef.h> // ptrdiff_t, size_t
Expand Down Expand Up @@ -37,37 +38,45 @@ namespace frontend {

class ObjectBox;

class CGBigIntList {
JS::Rooted<BigIntVector> vector;
struct MOZ_STACK_CLASS GCThingList {
JS::RootedVector<StackGCCellPtr> vector;

public:
explicit CGBigIntList(JSContext* cx) : vector(cx, BigIntVector(cx)) {}
MOZ_MUST_USE bool append(js::BigInt* bi) { return vector.append(bi); }
size_t length() const { return vector.length(); }
void finish(mozilla::Span<GCPtrBigInt> array);
};

struct CGObjectList {
// Number of emitted so far objects.
uint32_t length;
// Last emitted object.
ObjectBox* lastbox;
ObjectBox* lastbox = nullptr;

CGObjectList() : length(0), lastbox(nullptr) {}
// Index of the first scope in the vector.
mozilla::Maybe<uint32_t> firstScopeIndex;

unsigned add(ObjectBox* objbox);
void finish(mozilla::Span<GCPtrObject> array);
void finishInnerFunctions();
};

struct MOZ_STACK_CLASS CGScopeList {
JS::Rooted<GCVector<Scope*>> vector;
explicit GCThingList(JSContext* cx) : vector(cx) {}

explicit CGScopeList(JSContext* cx) : vector(cx, GCVector<Scope*>(cx)) {}
MOZ_MUST_USE bool append(Scope* scope, uint32_t* index) {
*index = vector.length();
if (!vector.append(JS::GCCellPtr(scope))) {
return false;
}
if (!firstScopeIndex) {
firstScopeIndex.emplace(*index);
}
return true;
}
MOZ_MUST_USE bool append(BigInt* bi, uint32_t* index) {
*index = vector.length();
return vector.append(JS::GCCellPtr(bi));
}
MOZ_MUST_USE bool append(ObjectBox* obj, uint32_t* index);

bool append(Scope* scope) { return vector.append(scope); }
uint32_t length() const { return vector.length(); }
void finish(mozilla::Span<GCPtrScope> array);
void finish(mozilla::Span<JS::GCCellPtr> array);
void finishInnerFunctions();

Scope* getScope(size_t index) const {
return &vector[index].get().get().as<Scope>();
}

Scope* firstScope() const {
MOZ_ASSERT(firstScopeIndex.isSome());
return getScope(*firstScopeIndex);
}
};

struct CGTryNoteList {
Expand Down Expand Up @@ -325,35 +334,15 @@ class PerScriptData {

MOZ_MUST_USE bool init(JSContext* cx);

// ---- Scope ----

CGScopeList& scopeList() { return scopeList_; }
const CGScopeList& scopeList() const { return scopeList_; }

// ---- Literals ----

CGBigIntList& bigIntList() { return bigIntList_; }
const CGBigIntList& bigIntList() const { return bigIntList_; }

CGObjectList& objectList() { return objectList_; }
const CGObjectList& objectList() const { return objectList_; }
GCThingList& gcThingList() { return gcThingList_; }
const GCThingList& gcThingList() const { return gcThingList_; }

PooledMapPtr<AtomIndexMap>& atomIndices() { return atomIndices_; }
const PooledMapPtr<AtomIndexMap>& atomIndices() const { return atomIndices_; }

private:
// ---- Scope ----

// List of emitted scopes.
CGScopeList scopeList_;

// ---- Literals ----

// List of bigints used by script.
CGBigIntList bigIntList_;

// List of emitted objects.
CGObjectList objectList_;
// List of emitted scopes/objects/bigints.
GCThingList gcThingList_;

// Map from atom to index.
PooledMapPtr<AtomIndexMap> atomIndices_;
Expand Down
7 changes: 3 additions & 4 deletions js/src/frontend/EmitterScope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,16 +342,15 @@ bool EmitterScope::internScope(BytecodeEmitter* bce, ScopeCreator createScope) {
return false;
}
hasEnvironment_ = scope->hasEnvironment();
scopeIndex_ = bce->perScriptData().scopeList().length();
return bce->perScriptData().scopeList().append(scope);
return bce->perScriptData().gcThingList().append(scope, &scopeIndex_);
}

template <typename ScopeCreator>
bool EmitterScope::internBodyScope(BytecodeEmitter* bce,
ScopeCreator createScope) {
MOZ_ASSERT(bce->bodyScopeIndex == UINT32_MAX,
"There can be only one body scope");
bce->bodyScopeIndex = bce->perScriptData().scopeList().length();
bce->bodyScopeIndex = bce->perScriptData().gcThingList().length();
return internScope(bce, createScope);
}

Expand Down Expand Up @@ -1071,7 +1070,7 @@ bool EmitterScope::leave(BytecodeEmitter* bce, bool nonLocal) {
}

Scope* EmitterScope::scope(const BytecodeEmitter* bce) const {
return bce->perScriptData().scopeList().vector[index()];
return bce->perScriptData().gcThingList().getScope(index());
}

NameLocation EmitterScope::lookup(BytecodeEmitter* bce, JSAtom* name) {
Expand Down
Loading

0 comments on commit 7542b54

Please sign in to comment.