Changeset 254419 in webkit

Timestamp:

Jan 12, 2020 7:10:13 PM (4 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] Remove IsDone from JSArrayIterator
​https://bugs.webkit.org/show_bug.cgi?id=206140

Reviewed by Keith Miller.

We can store -1 in Index field to represent whether the iterator is closed.
While this patch does not change the allocation size of JSArrayIterator, this style can
shrink the size of JSStringIterator when we implement it in the same style.

We also rename iterationKindKeyValue to iterationKindEntries.

• builtins/ArrayIteratorPrototype.js:

(globalPrivate.arrayIteratorNextHelper):

• builtins/MapIteratorPrototype.js:

(globalPrivate.mapIteratorNext):

• builtins/MapPrototype.js:

(entries):

• builtins/SetIteratorPrototype.js:

(globalPrivate.setIteratorNext):

• builtins/SetPrototype.js:

(entries):

• bytecode/BytecodeIntrinsicRegistry.cpp:

(JSC::BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry):

• bytecode/BytecodeIntrinsicRegistry.h:
• bytecompiler/NodesCodegen.cpp:

(JSC::arrayIteratorInternalFieldIndex):

• inspector/JSInjectedScriptHost.cpp:

(Inspector::cloneArrayIteratorObject):

• runtime/JSArrayIterator.cpp:

(JSC::JSArrayIterator::finishCreation):

• runtime/JSArrayIterator.h:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• builtins/ArrayIteratorPrototype.js (modified) (2 diffs)
• builtins/MapIteratorPrototype.js (modified) (1 diff)
• builtins/MapPrototype.js (modified) (1 diff)
• builtins/SetIteratorPrototype.js (modified) (1 diff)
• builtins/SetPrototype.js (modified) (1 diff)
• bytecode/BytecodeIntrinsicRegistry.cpp (modified) (2 diffs)
• bytecode/BytecodeIntrinsicRegistry.h (modified) (1 diff)
• bytecompiler/NodesCodegen.cpp (modified) (1 diff)
• inspector/JSInjectedScriptHost.cpp (modified) (1 diff)
• runtime/JSArrayIterator.cpp (modified) (1 diff)
• runtime/JSArrayIterator.h (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-01-12  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Remove IsDone from JSArrayIterator
+        https://bugs.webkit.org/show_bug.cgi?id=206140
+
+        Reviewed by Keith Miller.
+
+        We can store `-1` in Index field to represent whether the iterator is closed.
+        While this patch does not change the allocation size of JSArrayIterator, this style can
+        shrink the size of JSStringIterator when we implement it in the same style.
+
+        We also rename iterationKindKeyValue to iterationKindEntries.
+
+        * builtins/ArrayIteratorPrototype.js:
+        (globalPrivate.arrayIteratorNextHelper):
+        * builtins/MapIteratorPrototype.js:
+        (globalPrivate.mapIteratorNext):
+        * builtins/MapPrototype.js:
+        (entries):
+        * builtins/SetIteratorPrototype.js:
+        (globalPrivate.setIteratorNext):
+        * builtins/SetPrototype.js:
+        (entries):
+        * bytecode/BytecodeIntrinsicRegistry.cpp:
+        (JSC::BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry):
+        * bytecode/BytecodeIntrinsicRegistry.h:
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::arrayIteratorInternalFieldIndex):
+        * inspector/JSInjectedScriptHost.cpp:
+        (Inspector::cloneArrayIteratorObject):
+        * runtime/JSArrayIterator.cpp:
+        (JSC::JSArrayIterator::finishCreation):
+        * runtime/JSArrayIterator.h:
+
 2020-01-12  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/builtins/ArrayIteratorPrototype.js

@@ -46 +46 @@
     var value;
 
-    if (!@getArrayIteratorInternalField(this, @arrayIteratorFieldIsDone)) {
-        var index = @getArrayIteratorInternalField(this, @arrayIteratorFieldIndex);
+    var index = @getArrayIteratorInternalField(this, @arrayIteratorFieldIndex);
+    if (index !== -1) {
         var length = array.length >>> 0;
         if (index < length) {
@@ -54 +54 @@
             if (kind === @iterationKindKey)
                 value = index;
-            else { 
+            else {
                 value = array[index];
-                value = kind === @iterationKindValue ? value : [index, value];
+                if (kind === @iterationKindEntries)
+                    value = [index, value];
             }
-        }
+        } else
+            @putArrayIteratorInternalField(this, @arrayIteratorFieldIndex, -1);
     }
-    @putArrayIteratorInternalField(this, @arrayIteratorFieldIsDone, done);
 
     return { value, done };

trunk/Source/JavaScriptCore/builtins/MapIteratorPrototype.js

@@ -37 +37 @@
         var key = @mapBucketKey(bucket);
         value = @mapBucketValue(bucket);
-        if (kind === @iterationKindKeyValue)
+        if (kind === @iterationKindEntries)
             value = [ key, value ]
         else if (kind === @iterationKindKey)

trunk/Source/JavaScriptCore/builtins/MapPrototype.js

@@ -63 +63 @@
         @throwTypeError("Map.prototype.entries requires that |this| be Map");
 
-    return new @MapIterator(this, @iterationKindKeyValue);
+    return new @MapIterator(this, @iterationKindEntries);
 }
 

trunk/Source/JavaScriptCore/builtins/SetIteratorPrototype.js

@@ -36 +36 @@
     if (!done) {
         value = @setBucketKey(bucket);
-        if (kind === @iterationKindKeyValue)
+        if (kind === @iterationKindEntries)
             value = [ value, value ]
     }

trunk/Source/JavaScriptCore/builtins/SetPrototype.js

@@ -53 +53 @@
         @throwTypeError("Set.prototype.entries requires that |this| be Set");
 
-    return new @SetIterator(this, @iterationKindKeyValue);
+    return new @SetIterator(this, @iterationKindEntries);
 }
 

trunk/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.cpp

@@ -59 +59 @@
     m_iterationKindKey.set(m_vm, jsNumber(static_cast<unsigned>(IterationKind::Keys)));
     m_iterationKindValue.set(m_vm, jsNumber(static_cast<unsigned>(IterationKind::Values)));
-    m_iterationKindKeyValue.set(m_vm, jsNumber(static_cast<unsigned>(IterationKind::Entries)));
+    m_iterationKindEntries.set(m_vm, jsNumber(static_cast<unsigned>(IterationKind::Entries)));
     m_MAX_ARRAY_INDEX.set(m_vm, jsNumber(MAX_ARRAY_INDEX));
     m_MAX_STRING_LENGTH.set(m_vm, jsNumber(JSString::MaxLength));
@@ -91 +91 @@
     m_arrayIteratorFieldIteratedObject.set(m_vm, jsNumber(static_cast<int32_t>(JSArrayIterator::Field::IteratedObject)));
     m_arrayIteratorFieldIndex.set(m_vm, jsNumber(static_cast<int32_t>(JSArrayIterator::Field::Index)));
-    m_arrayIteratorFieldIsDone.set(m_vm, jsNumber(static_cast<int32_t>(JSArrayIterator::Field::IsDone)));
     m_arrayIteratorFieldKind.set(m_vm, jsNumber(static_cast<int32_t>(JSArrayIterator::Field::Kind)));
     m_asyncGeneratorFieldSuspendReason.set(m_vm, jsNumber(static_cast<unsigned>(JSAsyncGenerator::Field::SuspendReason)));

trunk/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.h

@@ -90 +90 @@
     macro(iterationKindKey) \
     macro(iterationKindValue) \
-    macro(iterationKindKeyValue) \
+    macro(iterationKindEntries) \
     macro(MAX_ARRAY_INDEX) \
     macro(MAX_STRING_LENGTH) \

trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

@@ -1070 +1070 @@
     if (node->entry().emitter() == &BytecodeIntrinsicNode::emit_intrinsic_arrayIteratorFieldIndex)
         return JSArrayIterator::Field::Index;
-    if (node->entry().emitter() == &BytecodeIntrinsicNode::emit_intrinsic_arrayIteratorFieldIsDone)
-        return JSArrayIterator::Field::IsDone;
     if (node->entry().emitter() == &BytecodeIntrinsicNode::emit_intrinsic_arrayIteratorFieldIteratedObject)
         return JSArrayIterator::Field::IteratedObject;

trunk/Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp

@@ -547 +547 @@
     JSArrayIterator* clone = JSArrayIterator::create(vm, globalObject->arrayIteratorStructure(), iteratorObject->iteratedObject(), iteratorObject->internalField(JSArrayIterator::Field::Kind).get());
     clone->internalField(JSArrayIterator::Field::Index).set(vm, clone, iteratorObject->internalField(JSArrayIterator::Field::Index).get());
-    clone->internalField(JSArrayIterator::Field::IsDone).set(vm, clone, iteratorObject->internalField(JSArrayIterator::Field::IsDone).get());
     return clone;
 }

trunk/Source/JavaScriptCore/runtime/JSArrayIterator.cpp

@@ -63 +63 @@
 void JSArrayIterator::finishCreation(VM& vm)
 {
+    Base::finishCreation(vm);
     auto values = initialValues();
     for (unsigned index = 0; index < values.size(); ++index)
         Base::internalField(index).set(vm, this, values[index]);
-    Base::finishCreation(vm);
 }
 

trunk/Source/JavaScriptCore/runtime/JSArrayIterator.h

@@ -36 +36 @@
     enum class Field : uint8_t { 
         Index = 0,
-        IsDone,
         IteratedObject,
         Kind,
     };
 
-    // JSArrayIterator has one inline storage slot, which is pointing internalField(0).
     static size_t allocationSize(Checked<size_t> inlineCapacity)
     {
@@ -58 +56 @@
         return { {
             jsNumber(0),
-            jsBoolean(false),
             jsNull(),
             jsNumber(0),