Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JSC] Add NoIndexing miss optimization
https://bugs.webkit.org/show_bug.cgi?id=240290
rdar://problem/93456330

Reviewed by Saam Barati.

`Array.from({ length: 42 })` idiom becomes common. And this is currently inefficient since we
perform missed indexed access of the object 42 times. But if we do not have indexed properties
for this object and it is plain normal object, then we can have optimization folding it to undefined
as we are skipping prototype-chain access for OOB array access.

This patch introduces IndexedNoIndexingMiss IC. It guards via structures, and they are ensured that
they do not have indexing types. To represent it in PropertyCondition, this patch addds AbsenceOfIndexedProperties
PropertyCondition. When we found NoIndexing, we attempt to create IC by collecting structures.

In the future, we would like to further optimize it in DFG and FTL by using this condition. But for now,
we just use IC. We also add simple optimization via AI-proven structures.

PropertyCondition AbsenceOfIndexedProperties is useful, and in the future patches, we should use it to ensure
that iterator protocol is met more efficiently.

Attached benchmark gets 7x performance improvement.

                                   ToT                     Patched

array-from-object           697.6093+-0.9029     ^     89.7427+-0.7996        ^ definitely 7.7734x faster
array-from-object-func      698.9900+-0.5077     ^    103.7614+-1.7910        ^ definitely 6.7365x faster

* JSTests/microbenchmarks/array-from-derived-object-func.js: Added.
(shouldBe):
(Derived):
* JSTests/microbenchmarks/array-from-object-func.js: Added.
(shouldBe):
* JSTests/microbenchmarks/array-from-object.js: Added.
(shouldBe):
* JSTests/stress/no-indexing-shape-invalidate.js: Added.
(shouldBe):
(test):
* JSTests/stress/no-indexing-shape-multiple.js: Added.
(shouldBe):
(test):
* JSTests/stress/no-indexing-shape-negative.js: Added.
(shouldBe):
(test):
* JSTests/stress/no-indexing-shape-other-array.js: Added.
(shouldBe):
(test):
* JSTests/stress/no-indexing-shape-prototype-invalidate.js: Added.
(shouldBe):
(test):
* Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:
(JSC::MacroAssemblerARM64::add8):
* Source/JavaScriptCore/bytecode/AccessCase.cpp:
(JSC::AccessCase::create):
(JSC::AccessCase::guardedByStructureCheckSkippingConstantIdentifierCheck const):
(JSC::AccessCase::requiresIdentifierNameMatch const):
(JSC::AccessCase::requiresInt32PropertyCheck const):
(JSC::AccessCase::needsScratchFPR const):
(JSC::AccessCase::forEachDependentCell const):
(JSC::AccessCase::doesCalls const):
(JSC::AccessCase::canReplace const):
(JSC::AccessCase::generateWithGuard):
(JSC::AccessCase::generateImpl):
(JSC::AccessCase::canBeShared):
* Source/JavaScriptCore/bytecode/AccessCase.h:
(JSC::AccessCase::create): Deleted.
* Source/JavaScriptCore/bytecode/ArrayProfile.h:
(JSC::ArrayProfile::ArrayProfile): Deleted.
* Source/JavaScriptCore/bytecode/ObjectPropertyCondition.h:
(JSC::ObjectPropertyCondition::absenceOfIndexedPropertiesWithoutBarrier):
(JSC::ObjectPropertyCondition::absenceOfIndexedProperties):
* Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp:
(JSC::generateConditionsForIndexedMiss):
* Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.h:
* Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp:
(JSC::PolymorphicAccess::regenerate):
(WTF::printInternal):
* Source/JavaScriptCore/bytecode/PropertyCondition.cpp:
(JSC::PropertyCondition::dumpInContext const):
(JSC::PropertyCondition::isStillValidAssumingImpurePropertyWatchpoint const):
(JSC::PropertyCondition::validityRequiresImpurePropertyWatchpoint const):
(JSC::PropertyCondition::isStillValid const):
(WTF::printInternal):
* Source/JavaScriptCore/bytecode/PropertyCondition.h:
(JSC::PropertyCondition::absenceOfIndexedPropertiesWithoutBarrier):
(JSC::PropertyCondition::absenceOfIndexedProperties):
(JSC::PropertyCondition::hasPrototype const):
(JSC::PropertyCondition::hash const):
(JSC::PropertyCondition::operator== const):
* Source/JavaScriptCore/bytecode/Repatch.cpp:
(JSC::tryCacheArrayGetByVal):

Canonical link: https://commits.webkit.org/253120@main
  • Loading branch information
Constellation committed Aug 4, 2022
1 parent 1f03945 commit 801dcc1
Show file tree
Hide file tree
Showing 19 changed files with 295 additions and 25 deletions.
20 changes: 20 additions & 0 deletions JSTests/microbenchmarks/array-from-derived-object-func.js
@@ -0,0 +1,20 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

class Derived {
constructor()
{
this.length = 1024 * 1024 * 2;
}
}

let object = new Derived();
for (let i = 0; i < 30; i++) {
let array = Array.from(object, () => 42);
shouldBe(array.length, 1024 * 1024 * 2);
shouldBe(1 in array, true);
shouldBe(array[0], 42);
shouldBe(array[1], 42);
}
12 changes: 12 additions & 0 deletions JSTests/microbenchmarks/array-from-object-func.js
@@ -0,0 +1,12 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

for (let i = 0; i < 30; i++) {
let array = Array.from({ length: 1024 * 1024 * 2 }, () => 42);
shouldBe(array.length, 1024 * 1024 * 2);
shouldBe(1 in array, true);
shouldBe(array[0], 42);
shouldBe(array[1], 42);
}
12 changes: 12 additions & 0 deletions JSTests/microbenchmarks/array-from-object.js
@@ -0,0 +1,12 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

for (let i = 0; i < 30; i++) {
let array = Array.from({ length: 1024 * 1024 * 2 });
shouldBe(array.length, 1024 * 1024 * 2);
shouldBe(1 in array, true);
shouldBe(array[0], undefined);
shouldBe(array[1], undefined);
}
15 changes: 15 additions & 0 deletions JSTests/stress/no-indexing-shape-invalidate.js
@@ -0,0 +1,15 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

function test(object, index) {
return object[index];
}
noInline(test);

var object = {};
for (var i = 0; i < 1e3; ++i)
shouldBe(test(object, i), undefined);
Object.prototype[30] = 42;
shouldBe(test(object, 30), 42);
16 changes: 16 additions & 0 deletions JSTests/stress/no-indexing-shape-multiple.js
@@ -0,0 +1,16 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

function test(object, index) {
return object[index];
}
noInline(test);

let v1 = { length: 42 };
let v2 = { length: 42, test: 42 };
for (let i = 0; i < 1e6; ++i) {
shouldBe(test(v1, i), undefined);
shouldBe(test(v2, i), undefined);
}
15 changes: 15 additions & 0 deletions JSTests/stress/no-indexing-shape-negative.js
@@ -0,0 +1,15 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

function test(object, index) {
return object[index];
}
noInline(test);

Object.prototype[-30] = 42;
var object = {};
for (var i = 0; i < 1e3; ++i)
shouldBe(test(object, i), undefined);
shouldBe(test(object, -30), 42);
15 changes: 15 additions & 0 deletions JSTests/stress/no-indexing-shape-other-array.js
@@ -0,0 +1,15 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

function test(object, index) {
return object[index];
}
noInline(test);

var object = {};
object.__proto__ = new Uint8Array(0);
for (var i = 0; i < 1e3; ++i)
shouldBe(test(object, i), undefined);
shouldBe(test(object, 30), undefined);
19 changes: 19 additions & 0 deletions JSTests/stress/no-indexing-shape-prototype-invalidate.js
@@ -0,0 +1,19 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

function test(object, index) {
return object[index];
}
noInline(test);

var object = {};
for (var i = 0; i < 1e3; ++i)
shouldBe(test(object, i), undefined);

object.__proto__ = {
__proto__: Object.prototype,
[30]: 42
};
shouldBe(test(object, 30), 42);
7 changes: 7 additions & 0 deletions Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
Expand Up @@ -353,6 +353,13 @@ class MacroAssemblerARM64 : public AbstractMacroAssembler<Assembler> {
m_assembler.add<64>(dest, dest, dataTempRegister);
}

void add8(TrustedImm32 imm, Address address)
{
load8(address, getCachedMemoryTempRegisterIDAndInvalidate());
add32(imm, memoryTempRegister, getCachedDataTempRegisterIDAndInvalidate());
store8(dataTempRegister, address);
}

void and32(RegisterID src, RegisterID dest)
{
and32(dest, src, dest);
Expand Down
23 changes: 22 additions & 1 deletion Source/JavaScriptCore/bytecode/AccessCase.cpp
Expand Up @@ -99,6 +99,7 @@ Ref<AccessCase> AccessCase::create(VM& vm, JSCell* owner, AccessType type, Cache
case IndexedTypedArrayFloat32Load:
case IndexedTypedArrayFloat64Load:
case IndexedStringLoad:
case IndexedNoIndexingMiss:
case IndexedInt32Store:
case IndexedDoubleStore:
case IndexedContiguousStore:
Expand Down Expand Up @@ -333,6 +334,7 @@ bool AccessCase::guardedByStructureCheckSkippingConstantIdentifierCheck() const
case IndexedTypedArrayFloat32Store:
case IndexedTypedArrayFloat64Store:
return false;
case IndexedNoIndexingMiss:
default:
return true;
}
Expand Down Expand Up @@ -386,6 +388,7 @@ bool AccessCase::requiresIdentifierNameMatch() const
case IndexedTypedArrayFloat32Load:
case IndexedTypedArrayFloat64Load:
case IndexedStringLoad:
case IndexedNoIndexingMiss:
case IndexedInt32Store:
case IndexedDoubleStore:
case IndexedContiguousStore:
Expand Down Expand Up @@ -451,6 +454,7 @@ bool AccessCase::requiresInt32PropertyCheck() const
case IndexedTypedArrayFloat32Load:
case IndexedTypedArrayFloat64Load:
case IndexedStringLoad:
case IndexedNoIndexingMiss:
case IndexedInt32Store:
case IndexedDoubleStore:
case IndexedContiguousStore:
Expand Down Expand Up @@ -510,6 +514,7 @@ bool AccessCase::needsScratchFPR() const
case IndexedTypedArrayUint16Load:
case IndexedTypedArrayInt32Load:
case IndexedStringLoad:
case IndexedNoIndexingMiss:
case IndexedInt32Store:
case IndexedContiguousStore:
case IndexedArrayStorageStore:
Expand Down Expand Up @@ -615,6 +620,7 @@ void AccessCase::forEachDependentCell(VM&, const Functor& functor) const
case IndexedTypedArrayFloat32Load:
case IndexedTypedArrayFloat64Load:
case IndexedStringLoad:
case IndexedNoIndexingMiss:
case IndexedInt32Store:
case IndexedDoubleStore:
case IndexedContiguousStore:
Expand Down Expand Up @@ -682,6 +688,7 @@ bool AccessCase::doesCalls(VM& vm, Vector<JSCell*>* cellsToMarkIfDoesCalls) cons
case IndexedTypedArrayFloat32Load:
case IndexedTypedArrayFloat64Load:
case IndexedStringLoad:
case IndexedNoIndexingMiss:
case IndexedInt32Store:
case IndexedDoubleStore:
case IndexedContiguousStore:
Expand Down Expand Up @@ -837,6 +844,7 @@ bool AccessCase::canReplace(const AccessCase& other) const
case InMiss:
case CheckPrivateBrand:
case SetPrivateBrand:
case IndexedNoIndexingMiss:
if (other.type() != type())
return false;

Expand Down Expand Up @@ -1322,6 +1330,13 @@ void AccessCase::generateWithGuard(
return;
}

case IndexedNoIndexingMiss: {
emitDefaultGuard();
GPRReg propertyGPR = state.propertyGPR();
state.failAndIgnore.append(jit.branch32(CCallHelpers::LessThan, propertyGPR, CCallHelpers::TrustedImm32(0)));
break;
}

case IndexedInt32Load:
case IndexedDoubleLoad:
case IndexedContiguousLoad:
Expand Down Expand Up @@ -2493,7 +2508,12 @@ void AccessCase::generateImpl(AccessGenerationState& state)
state.succeed();
return;
}


case IndexedNoIndexingMiss:
jit.moveTrustedValue(jsUndefined(), valueRegs);
state.succeed();
return;

case IntrinsicGetter: {
RELEASE_ASSERT(isValidOffset(offset()));

Expand Down Expand Up @@ -2668,6 +2688,7 @@ bool AccessCase::canBeShared(const AccessCase& lhs, const AccessCase& rhs)
case IndexedTypedArrayFloat32Store:
case IndexedTypedArrayFloat64Store:
case IndexedStringLoad:
case IndexedNoIndexingMiss:
case InstanceOfGeneric:
return true;

Expand Down
7 changes: 1 addition & 6 deletions Source/JavaScriptCore/bytecode/AccessCase.h
Expand Up @@ -129,6 +129,7 @@ class AccessCase : public ThreadSafeRefCounted<AccessCase> {
IndexedTypedArrayFloat32Load,
IndexedTypedArrayFloat64Load,
IndexedStringLoad,
IndexedNoIndexingMiss,
IndexedInt32Store,
IndexedDoubleStore,
IndexedContiguousStore,
Expand Down Expand Up @@ -157,12 +158,6 @@ class AccessCase : public ThreadSafeRefCounted<AccessCase> {
const T& as() const { return *static_cast<const T*>(this); }


template<typename AccessCaseType, typename... Arguments>
static std::unique_ptr<AccessCaseType> create(Arguments... arguments)
{
return std::unique_ptr<AccessCaseType>(new AccessCaseType(arguments...));
}

static Ref<AccessCase> create(VM&, JSCell* owner, AccessType, CacheableIdentifier, PropertyOffset = invalidOffset,
Structure* = nullptr, const ObjectPropertyConditionSet& = ObjectPropertyConditionSet(), RefPtr<PolyProtoAccessChain>&& = nullptr);

Expand Down
13 changes: 4 additions & 9 deletions Source/JavaScriptCore/bytecode/ArrayProfile.h
Expand Up @@ -199,12 +199,7 @@ class ArrayProfile {
friend class UnlinkedArrayProfile;

public:
explicit ArrayProfile()
: m_mayInterceptIndexedAccesses(false)
, m_usesOriginalArrayStructures(true)
, m_didPerformFirstRunPruning(false)
{
}
explicit ArrayProfile() = default;

#if USE(LARGE_TYPED_ARRAYS)
static constexpr uint64_t s_smallTypedArrayMaxLength = std::numeric_limits<int32_t>::max();
Expand Down Expand Up @@ -255,9 +250,9 @@ class ArrayProfile {
#if USE(LARGE_TYPED_ARRAYS)
bool m_mayBeLargeTypedArray { false };
#endif
bool m_mayInterceptIndexedAccesses : 1;
bool m_usesOriginalArrayStructures : 1;
bool m_didPerformFirstRunPruning : 1;
bool m_mayInterceptIndexedAccesses : 1 { false };
bool m_usesOriginalArrayStructures : 1 { true };
bool m_didPerformFirstRunPruning : 1 { false };
ArrayModes m_observedArrayModes { 0 };
};
static_assert(sizeof(ArrayProfile) == 12);
Expand Down
17 changes: 16 additions & 1 deletion Source/JavaScriptCore/bytecode/ObjectPropertyCondition.h
Expand Up @@ -120,7 +120,22 @@ class ObjectPropertyCondition {
vm.writeBarrier(owner);
return absenceOfSetEffectWithoutBarrier(object, uid, prototype);
}


static ObjectPropertyCondition absenceOfIndexedPropertiesWithoutBarrier(JSObject* object, JSObject* prototype)
{
ObjectPropertyCondition result;
result.m_object = object;
result.m_condition = PropertyCondition::absenceOfIndexedPropertiesWithoutBarrier(prototype);
return result;
}

static ObjectPropertyCondition absenceOfIndexedProperties(VM& vm, JSCell* owner, JSObject* object, JSObject* prototype)
{
if (owner)
vm.writeBarrier(owner);
return absenceOfIndexedPropertiesWithoutBarrier(object, prototype);
}

static ObjectPropertyCondition equivalenceWithoutBarrier(
JSObject* object, UniquedStringImpl* uid, JSValue value)
{
Expand Down
24 changes: 24 additions & 0 deletions Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp
Expand Up @@ -219,6 +219,16 @@ ObjectPropertyCondition generateCondition(
vm, owner, object, uid, structure->storedPrototypeObject());
break;
}
case PropertyCondition::AbsenceOfIndexedProperties: {
if (structure->hasPolyProto())
return ObjectPropertyCondition();
if (hasIndexedProperties(structure->indexingType()))
return ObjectPropertyCondition();
if (structure->mayInterceptIndexedAccesses() || structure->typeInfo().interceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero())
return ObjectPropertyCondition();
result = ObjectPropertyCondition::absenceOfIndexedProperties(vm, owner, object, structure->storedPrototypeObject());
break;
}
case PropertyCondition::Equivalence: {
unsigned attributes;
PropertyOffset offset = structure->get(vm, concurrency, uid, attributes);
Expand Down Expand Up @@ -349,6 +359,20 @@ ObjectPropertyConditionSet generateConditionsForPropertySetterMiss(
});
}

ObjectPropertyConditionSet generateConditionsForIndexedMiss(VM& vm, JSCell* owner, JSGlobalObject* globalObject, Structure* headStructure)
{
return generateConditions(
globalObject, headStructure, nullptr,
[&](auto& conditions, JSObject* object, Structure* structure) -> bool {
ObjectPropertyCondition result =
generateCondition(vm, owner, object, structure, nullptr, PropertyCondition::AbsenceOfIndexedProperties, Concurrency::MainThread);
if (!result)
return false;
conditions.append(result);
return true;
});
}

ObjectPropertyConditionSet generateConditionsForPrototypePropertyHit(
VM& vm, JSCell* owner, JSGlobalObject* globalObject, Structure* headStructure, JSObject* prototype,
UniquedStringImpl* uid)
Expand Down
Expand Up @@ -170,6 +170,7 @@ ObjectPropertyConditionSet generateConditionsForPropertyMiss(
VM&, JSCell* owner, JSGlobalObject*, Structure* headStructure, UniquedStringImpl* uid);
ObjectPropertyConditionSet generateConditionsForPropertySetterMiss(
VM&, JSCell* owner, JSGlobalObject*, Structure* headStructure, UniquedStringImpl* uid);
ObjectPropertyConditionSet generateConditionsForIndexedMiss(VM&, JSCell* owner, JSGlobalObject*, Structure* headStructure);
ObjectPropertyConditionSet generateConditionsForPrototypePropertyHit(
VM&, JSCell* owner, JSGlobalObject*, Structure* headStructure, JSObject* prototype,
UniquedStringImpl* uid);
Expand Down
7 changes: 5 additions & 2 deletions Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp
Expand Up @@ -724,15 +724,15 @@ AccessGenerationResult PolymorphicAccess::regenerate(const GCSafeConcurrentJSLoc
// patch things if the countdown reaches zero. We increment the slow path count here to ensure
// that the slow path does not try to patch.
if (codeBlock->useDataIC()) {
#if CPU(X86) || CPU(X86_64)
#if CPU(X86) || CPU(X86_64) || CPU(ARM64)
jit.add8(CCallHelpers::TrustedImm32(1), CCallHelpers::Address(stubInfo.m_stubInfoGPR, StructureStubInfo::offsetOfCountdown()));
#else
jit.load8(CCallHelpers::Address(stubInfo.m_stubInfoGPR, StructureStubInfo::offsetOfCountdown()), state.scratchGPR);
jit.add32(CCallHelpers::TrustedImm32(1), state.scratchGPR);
jit.store8(state.scratchGPR, CCallHelpers::Address(stubInfo.m_stubInfoGPR, StructureStubInfo::offsetOfCountdown()));
#endif
} else {
#if CPU(X86) || CPU(X86_64)
#if CPU(X86) || CPU(X86_64) || CPU(ARM64)
jit.move(CCallHelpers::TrustedImmPtr(&stubInfo.countdown), state.scratchGPR);
jit.add8(CCallHelpers::TrustedImm32(1), CCallHelpers::Address(state.scratchGPR));
#else
Expand Down Expand Up @@ -1023,6 +1023,9 @@ void printInternal(PrintStream& out, AccessCase::AccessType type)
case AccessCase::IndexedStringLoad:
out.print("IndexedStringLoad");
return;
case AccessCase::IndexedNoIndexingMiss:
out.print("IndexedNoIndexingMiss");
return;
case AccessCase::IndexedInt32Store:
out.print("IndexedInt32Store");
return;
Expand Down

0 comments on commit 801dcc1

Please sign in to comment.