Skip to content
Permalink
Browse files
Convert for-of iteration to in-band signalling so we can trivially av…
…oid unnecessary object allocation

https://bugs.webkit.org/show_bug.cgi?id=122532

Reviewed by Michael Saboff.

Source/JavaScriptCore:

Switch for-of enumeration to use in band signalling to determine the end
of iteration.  This allows us to trivially remove an otherwise unnecessary
object allocation, and paves the way for optimised thunks in future.

We can re-add explicit .next() functions in future that would marshall
the true iteration functions, but for now we'll ignore them.

This results in a huge improvement in the performance of for-of (in the order
of 2x) but there's still a long way to go in order to get the performance to
a satisfactory level.

* bytecompiler/NodesCodegen.cpp:
(JSC::ForOfNode::emitBytecode):
* runtime/ArrayIteratorPrototype.cpp:
(JSC::ArrayIteratorPrototype::finishCreation):
(JSC::createIteratorResult):
* runtime/CommonIdentifiers.cpp:
(JSC::CommonIdentifiers::CommonIdentifiers):
* runtime/CommonIdentifiers.h:
* runtime/Identifier.cpp:
(JSC::Identifier::addSlowCase):
* runtime/JSObject.h:
(JSC::JSFinalObject::create):
* runtime/VM.cpp:
(JSC::VM::VM):
* runtime/VM.h:

LayoutTests:

Update tests to reflect our non-exposure of .next()

* js/array-iterators-expected.txt:
* js/script-tests/array-iterators.js:

Canonical link: https://commits.webkit.org/140619@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@157150 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
ojhunt committed Oct 9, 2013
1 parent 25113e0 commit d6a2453e4b77a0a11a7c2b0a7be8534679601a82
@@ -1,3 +1,15 @@
2013-10-08 Oliver Hunt <oliver@apple.com>

Convert for-of iteration to in-band signalling so we can trivially avoid unnecessary object allocation
https://bugs.webkit.org/show_bug.cgi?id=122532

Reviewed by Michael Saboff.

Update tests to reflect our non-exposure of .next()

* js/array-iterators-expected.txt:
* js/script-tests/array-iterators.js:

2013-10-08 Gustavo Noronha Silva <gns@gnome.org>

Unreviewed gardening. Make platform-specific expectation file apply only to gtk-wk1, since
@@ -10,15 +10,13 @@ PASS key is 3
PASS key is 4
PASS key is 5
PASS testArray.length is 6
PASS keys.next().value is undefined.
PASS value is 1
PASS value is 2
PASS value is 3
PASS value is 4
PASS value is 5
PASS value is 6
PASS testArray.length is 6
PASS values.next().value is undefined.
PASS value is testArray[key]
PASS key is 0
PASS value is 1
@@ -38,7 +36,6 @@ PASS value is testArray[key]
PASS key is 5
PASS value is 6
PASS testArray.length is 6
PASS entries.next().value is undefined.
PASS value is testArray[key]
PASS key is i
PASS value is testArray[key]
@@ -58,7 +55,6 @@ PASS key is i
PASS value is testArray[key]
PASS key is i
PASS testArray.length is 9
PASS entries.next().value is undefined.
PASS successfullyParsed is true

TEST COMPLETE
@@ -6,58 +6,36 @@ description(
var testArray = [1,2,3,4,5,6]
var keys = testArray.keys();
var i = 0;
while (true) {
var {done, value: key} = keys.next();
if (done)
break;
for (var key of keys) {
shouldBe("key", String(i))
i++;
}

shouldBe("testArray.length", String(i))

shouldBeUndefined("keys.next().value")

var values = testArray.values();
var i = 0;
while (true) {
var {done, value} = values.next();
if (done)
break;
for (var value of values) {
i++;
shouldBe("value", String(i) )
}

shouldBe("testArray.length", String(i))

shouldBeUndefined("values.next().value")

var entries = testArray.entries();
var i = 0;
do {
var {done, value: entry} = entries.next();
if (done)
break;
var [key, value] = entry;
for (var [key, value] of entries) {
shouldBe("value", "testArray[key]")
shouldBe("key", String(i))
i++
shouldBe("value", String(i))
} while (!done);
}

shouldBe("testArray.length", String(i))

shouldBeUndefined("entries.next().value")



var entries = testArray.entries();
var i = 0;
do {
var {done, value: entry} = entries.next();
if (done)
break;
var [key, value] = entry;
for (var [key, value] of entries) {
shouldBe("value", "testArray[key]")
shouldBe("key", "i")
i++
@@ -69,8 +47,6 @@ do {
delete testArray[4]
if (i == 5)
testArray[4] = 5
} while (!done);
}
shouldBe("testArray.length", String(i))

shouldBeUndefined("entries.next().value")

@@ -1,3 +1,37 @@
2013-10-08 Oliver Hunt <oliver@apple.com>

Convert for-of iteration to in-band signalling so we can trivially avoid unnecessary object allocation
https://bugs.webkit.org/show_bug.cgi?id=122532

Reviewed by Michael Saboff.

Switch for-of enumeration to use in band signalling to determine the end
of iteration. This allows us to trivially remove an otherwise unnecessary
object allocation, and paves the way for optimised thunks in future.

We can re-add explicit .next() functions in future that would marshall
the true iteration functions, but for now we'll ignore them.

This results in a huge improvement in the performance of for-of (in the order
of 2x) but there's still a long way to go in order to get the performance to
a satisfactory level.

* bytecompiler/NodesCodegen.cpp:
(JSC::ForOfNode::emitBytecode):
* runtime/ArrayIteratorPrototype.cpp:
(JSC::ArrayIteratorPrototype::finishCreation):
(JSC::createIteratorResult):
* runtime/CommonIdentifiers.cpp:
(JSC::CommonIdentifiers::CommonIdentifiers):
* runtime/CommonIdentifiers.h:
* runtime/Identifier.cpp:
(JSC::Identifier::addSlowCase):
* runtime/JSObject.h:
(JSC::JSFinalObject::create):
* runtime/VM.cpp:
(JSC::VM::VM):
* runtime/VM.h:

2013-10-08 Alex Christensen <achristensen@webkit.org>

Fixed compile errors while compiling without the JIT enabled.
@@ -1778,7 +1778,7 @@ void ForOfNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
generator.emitMove(args.thisRegister(), subject.get());
generator.emitCall(iterator.get(), iterator.get(), NoExpectedFunction, args, divot(), divotStart(), divotEnd());
}
RefPtr<RegisterID> iteratorNext = generator.emitGetById(generator.newTemporary(), iterator.get(), generator.propertyNames().next);
RefPtr<RegisterID> iteratorNext = generator.emitGetById(generator.newTemporary(), iterator.get(), generator.propertyNames().iteratorNextPrivateName);
RefPtr<RegisterID> value = generator.newTemporary();
generator.emitLoad(value.get(), jsUndefined());

@@ -1822,14 +1822,13 @@ void ForOfNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
generator.emitNode(dst, m_statement);

generator.emitLabel(scope->continueTarget());
RefPtr<RegisterID> result = generator.newTemporary();
CallArguments nextArguments(generator, 0, 1);
generator.emitMove(nextArguments.thisRegister(), iterator.get());
generator.emitMove(nextArguments.argumentRegister(0), value.get());
generator.emitCall(result.get(), iteratorNext.get(), NoExpectedFunction, nextArguments, divot(), divotStart(), divotEnd());
generator.emitGetById(value.get(), result.get(), generator.propertyNames().value);
RefPtr<RegisterID> done = generator.emitGetById(generator.newTemporary(), result.get(), generator.propertyNames().done);
generator.emitJumpIfFalse(done.get(), loopStart.get());
generator.emitCall(value.get(), iteratorNext.get(), NoExpectedFunction, nextArguments, divot(), divotStart(), divotEnd());
RefPtr<RegisterID> result = generator.emitLoad(generator.newTemporary(), JSValue(generator.vm()->iterationTerminator.get()));
generator.emitJumpIfFalse(generator.emitEqualityOp(op_stricteq, result.get(), value.get(), result.get()), loopStart.get());
generator.emitLoad(value.get(), jsUndefined());
generator.emitDebugHook(WillExecuteStatement, firstLine(), startOffset(), lineStartOffset());
generator.emitLabel(scope->breakTarget());
}
@@ -45,38 +45,35 @@ void ArrayIteratorPrototype::finishCreation(VM& vm, JSGlobalObject* globalObject
ASSERT(inherits(info()));
vm.prototypeMap.addPrototype(this);

JSC_NATIVE_FUNCTION(vm.propertyNames->next, arrayIteratorPrototypeNext, DontEnum, 0);
JSC_NATIVE_FUNCTION(vm.propertyNames->iteratorNextPrivateName, arrayIteratorPrototypeNext, DontEnum, 0);
JSC_NATIVE_FUNCTION(vm.propertyNames->iteratorPrivateName, arrayIteratorPrototypeIterate, DontEnum, 0);
}

static EncodedJSValue createIteratorResult(CallFrame* callFrame, ArrayIterationKind kind, size_t index, JSValue result, bool done)
{
JSGlobalObject* globalObject = callFrame->callee()->globalObject();
JSObject* resultObject = constructEmptyObject(callFrame);
resultObject->putDirect(callFrame->vm(), callFrame->propertyNames().done, jsBoolean(done));
callFrame->setArgument(callFrame->argumentCount() - 1, jsBoolean(done));
if (done)
return JSValue::encode(callFrame->vm().iterationTerminator.get());

switch (kind & ~ArrayIterateSparseTag) {
case ArrayIterateKey:
resultObject->putDirect(callFrame->vm(), callFrame->propertyNames().value, done ? jsUndefined() : jsNumber(index));
break;
return JSValue::encode(jsNumber(index));

case ArrayIterateValue:
resultObject->putDirect(callFrame->vm(), callFrame->propertyNames().value, done ? jsUndefined() : result);
break;
return JSValue::encode(result);

case ArrayIterateKeyValue: {
if (!done) {
MarkedArgumentBuffer args;
args.append(jsNumber(index));
args.append(result);
JSArray* resultArray = constructArray(callFrame, 0, globalObject, args);
resultObject->putDirect(callFrame->vm(), callFrame->propertyNames().value, resultArray);
} else
resultObject->putDirect(callFrame->vm(), callFrame->propertyNames().value, jsUndefined());

break;
MarkedArgumentBuffer args;
args.append(jsNumber(index));
args.append(result);
JSGlobalObject* globalObject = callFrame->callee()->globalObject();
return JSValue::encode(constructArray(callFrame, 0, globalObject, args));

}
default:
RELEASE_ASSERT_NOT_REACHED();
}
return JSValue::encode(resultObject);
return JSValue::encode(JSValue());
}

EncodedJSValue JSC_HOST_CALL arrayIteratorPrototypeNext(CallFrame* callFrame)
@@ -35,6 +35,7 @@ CommonIdentifiers::CommonIdentifiers(VM* vm)
, thisIdentifier(vm, "this")
, useStrictIdentifier(vm, "use strict")
, iteratorPrivateName(Identifier::from(PrivateName()))
, iteratorNextPrivateName(Identifier::from(PrivateName()))
, hasNextIdentifier(vm, "hasNext")
JSC_COMMON_IDENTIFIERS_EACH_KEYWORD(INITIALIZE_KEYWORD)
JSC_COMMON_IDENTIFIERS_EACH_PROPERTY_NAME(INITIALIZE_PROPERTY_NAME)
@@ -213,6 +213,7 @@ namespace JSC {
const Identifier thisIdentifier;
const Identifier useStrictIdentifier;
const Identifier iteratorPrivateName;
const Identifier iteratorNextPrivateName;
const Identifier hasNextIdentifier;


@@ -132,6 +132,8 @@ PassRefPtr<StringImpl> Identifier::add8(VM* vm, const UChar* s, int length)

PassRefPtr<StringImpl> Identifier::addSlowCase(VM* vm, StringImpl* r)
{
if (r->isEmptyUnique())
return r;
ASSERT(!r->isIdentifier());
// The empty & null strings are static singletons, and static strings are handled
// in ::add() in the header, so we should never get here with a zero length string.
@@ -1033,6 +1033,7 @@ class JSFinalObject : public JSObject {
}

static JSFinalObject* create(ExecState*, Structure*);
static JSFinalObject* create(VM&, Structure*);
static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype, unsigned inlineCapacity)
{
return Structure::create(vm, globalObject, prototype, TypeInfo(FinalObjectType, StructureFlags), info(), NonArray, inlineCapacity);
@@ -1076,6 +1077,13 @@ inline JSFinalObject* JSFinalObject::create(ExecState* exec, Structure* structur
return finalObject;
}

inline JSFinalObject* JSFinalObject::create(VM& vm, Structure* structure)
{
JSFinalObject* finalObject = new (NotNull, allocateCell<JSFinalObject>(vm.heap, allocationSize(structure->inlineCapacity()))) JSFinalObject(vm, structure);
finalObject->finishCreation(vm);
return finalObject;
}

inline bool isJSFinalObject(JSCell* cell)
{
return cell->classInfo() == JSFinalObject::info();
@@ -245,7 +245,7 @@ VM::VM(VMType vmType, HeapType heapType)
propertyTableStructure.set(*this, PropertyTable::createStructure(*this, 0, jsNull()));
mapDataStructure.set(*this, MapData::createStructure(*this, 0, jsNull()));
weakMapDataStructure.set(*this, WeakMapData::createStructure(*this, 0, jsNull()));

iterationTerminator.set(*this, JSFinalObject::create(*this, JSFinalObject::createStructure(*this, 0, jsNull(), 1)));
smallStrings.initializeCommonStrings(*this);

wtfThreadData().setCurrentIdentifierTable(existingEntryIdentifierTable);
@@ -270,6 +270,7 @@ namespace JSC {
Strong<Structure> propertyTableStructure;
Strong<Structure> mapDataStructure;
Strong<Structure> weakMapDataStructure;
Strong<JSCell> iterationTerminator;

IdentifierTable* identifierTable;
CommonIdentifiers* propertyNames;

0 comments on commit d6a2453

Please sign in to comment.