From 76c588a5a8219e9ca69f4543041b2e82faceef09 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Thu, 1 Oct 2026 13:55:43 +0000 Subject: [PATCH 1/2] [JSC] Array profiles should not turn ordinary arrays into `ArrayStorage` https://bugs.webkit.org/show_bug.cgi?id=325781 Reviewed by NOBODY (OOPS!). Freezing an array turns it into ArrayStorage. After that, two profiles start to turn ordinary arrays into ArrayStorage, and those arrays miss most of the array fast paths. 1. ArrayMode::fromObserved picks ArrayStorage with Array::Convert for a site that has seen both Contiguous and ArrayStorage. Arrayify then converts every array that comes to the site. 2. ArrayAllocationProfile picks ArrayStorage for a site when the last array allocated there has become ArrayStorage. The array builtins share their profiles in a realm. Passing one frozen array to `every`, or freezing one result of `map`, affects every caller. This patch stops that. 1. ArrayMode::fromObserved ignores ArrayStorage when the site has seen Undecided, Int32, Double or Contiguous too. If an ArrayStorage array then fails the check in optimized code, ArrayProfile remembers it and the next compilation uses Array::Generic. 2. ArrayAllocationProfile ignores the last array when it has become ArrayStorage. The site keeps allocating arrays as before, and only the arrays that need ArrayStorage are converted later. 3. Arrayify exits with SparseIndex instead of Uncountable when the index is sparse, and ArrayMode::refine uses Array::Generic for the site in the next compilation. A site that writes to a sparse index did not reach this exit when Arrayify converted the array to ArrayStorage first. The other sites already kept exiting here. Both profiles still pick SlowPutArrayStorage as before. Every array has it when having a bad time. Some JetStream 3 tests hit this. Counts in one run, before -> after: Arrayify to ArrayStorage Allocated as ArrayStorage proxy-mobx 23306 -> 0 219027 -> 0 babel-wtb 53886 -> 0 21265 -> 0 jsdom-d3-startup 3011 -> 0 20834 -> 0 These tests were neutral when I ran them locally. Tests: JSTests/stress/array-allocation-profile-should-not-select-array-storage.js JSTests/stress/array-should-not-be-converted-to-array-storage-by-frequent-frozen-arrays.js JSTests/stress/array-should-not-be-converted-to-array-storage-by-frozen-array.js JSTests/stress/array-without-elements-should-not-be-converted-to-array-storage-by-frozen-array.js JSTests/stress/arrayify-should-not-keep-exiting-for-sparse-index.js * JSTests/stress/array-allocation-profile-should-not-select-array-storage.js: Added. (shouldBe): (createWithPush): (createWithLiteral): (createWithNewArray): (createWithMap): (createDoubleWithPush): (createInt32WithNewArray): (createInt32WithConstantLiteral): (createEmpty): (test): * JSTests/stress/array-should-not-be-converted-to-array-storage-by-frequent-frozen-arrays.js: Added. (shouldBe): (create): (createFrozen): (isObject): (every): (some): (filter): (forEach): (at): (indexed): (forOf): (destructure): (store): (push): (test): * JSTests/stress/array-should-not-be-converted-to-array-storage-by-frozen-array.js: Added. (shouldBe): (create): (createFrozen): (isObject): (every): (some): (filter): (forEach): (at): (indexed): (forOf): (destructure): (store): (push): (test): * JSTests/stress/array-without-elements-should-not-be-converted-to-array-storage-by-frozen-array.js: Added. (shouldBe): (shouldNotBeCompiledRepeatedly): (first): (indexed): (forOf): (destructure): (store): (push): (test): * JSTests/stress/arrayify-should-not-keep-exiting-for-sparse-index.js: Added. (shouldBe): (shouldNotBeCompiledRepeatedly): (putAfterOptimized): (putOncePerArray): (putToArrayBecomingSparse): (read): * Source/JavaScriptCore/bytecode/ArrayAllocationProfile.cpp: (JSC::ArrayAllocationProfile::updateProfile): * Source/JavaScriptCore/bytecode/ArrayProfile.cpp: (JSC::ArrayProfile::computeUpdatedPrediction): * Source/JavaScriptCore/bytecode/ArrayProfile.h: (JSC::ArrayProfile::removeObservedArrayModes): (JSC::ArrayProfile::speculationFailedOnArrayStorage const): * Source/JavaScriptCore/bytecode/ExitKind.h: * Source/JavaScriptCore/dfg/DFGArrayMode.cpp: (JSC::DFG::ArrayMode::fromObserved): (JSC::DFG::ArrayMode::refine const): * Source/JavaScriptCore/dfg/DFGArrayifySlowPathGenerator.h: * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::handleIteratorNext): * Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileArrayify): --- ...profile-should-not-select-array-storage.js | 85 +++++++++++++++++ ...array-storage-by-frequent-frozen-arrays.js | 92 +++++++++++++++++++ ...verted-to-array-storage-by-frozen-array.js | 92 +++++++++++++++++++ ...verted-to-array-storage-by-frozen-array.js | 73 +++++++++++++++ ...hould-not-keep-exiting-for-sparse-index.js | 73 +++++++++++++++ .../bytecode/ArrayAllocationProfile.cpp | 3 +- .../JavaScriptCore/bytecode/ArrayProfile.cpp | 8 +- Source/JavaScriptCore/bytecode/ArrayProfile.h | 3 + Source/JavaScriptCore/bytecode/ExitKind.h | 1 + Source/JavaScriptCore/dfg/DFGArrayMode.cpp | 19 +++- .../dfg/DFGArrayifySlowPathGenerator.h | 2 +- .../JavaScriptCore/dfg/DFGByteCodeParser.cpp | 12 ++- Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp | 2 +- 13 files changed, 455 insertions(+), 10 deletions(-) create mode 100644 JSTests/stress/array-allocation-profile-should-not-select-array-storage.js create mode 100644 JSTests/stress/array-should-not-be-converted-to-array-storage-by-frequent-frozen-arrays.js create mode 100644 JSTests/stress/array-should-not-be-converted-to-array-storage-by-frozen-array.js create mode 100644 JSTests/stress/array-without-elements-should-not-be-converted-to-array-storage-by-frozen-array.js create mode 100644 JSTests/stress/arrayify-should-not-keep-exiting-for-sparse-index.js diff --git a/JSTests/stress/array-allocation-profile-should-not-select-array-storage.js b/JSTests/stress/array-allocation-profile-should-not-select-array-storage.js new file mode 100644 index 0000000000000..420bf9ce3eeb2 --- /dev/null +++ b/JSTests/stress/array-allocation-profile-should-not-select-array-storage.js @@ -0,0 +1,85 @@ +//@ $skipModes << :lockdown + +function shouldBe(actual, expected) { + if (actual !== expected) + throw new Error(`expected ${expected} but got ${actual}`); +} + +function createWithPush() { + let array = []; + for (let i = 0; i < 8; ++i) + array.push({ i }); + return array; +} + +function createWithLiteral() { + return [{}, {}, {}]; +} + +function createWithNewArray() { + let array = new Array(8); + for (let i = 0; i < 8; ++i) + array[i] = { i }; + return array; +} + +let source = createWithPush(); + +function createWithMap() { + return source.map((value) => value); +} + +function createWithFilter() { + return source.filter(() => true); +} + +function createInt32WithPush() { + let array = []; + for (let i = 0; i < 8; ++i) + array.push(i); + return array; +} + +function createDoubleWithPush() { + let array = []; + for (let i = 0; i < 8; ++i) + array.push(i + 0.5); + return array; +} + +function createInt32WithNewArray() { + let array = new Array(8); + for (let i = 0; i < 8; ++i) + array[i] = i; + return array; +} + +function createInt32WithConstantLiteral() { + return [1, 2, 3]; +} + +function createEmpty() { + return []; +} + +function test(create, expected) { + for (let i = 0; i < testLoopCount; ++i) { + let array = create(); + if (!(i % 100)) + Object.freeze(array); + } + fullGC(); + for (let i = 0; i < testLoopCount; ++i) + shouldBe($vm.indexingMode(create()), expected); +} + +test(createWithPush, "ArrayWithContiguous"); +test(createWithLiteral, "ArrayWithContiguous"); +test(createWithNewArray, "ArrayWithContiguous"); +test(createWithMap, "ArrayWithContiguous"); +test(createWithFilter, "ArrayWithContiguous"); +test(createInt32WithPush, "ArrayWithInt32"); +test(createDoubleWithPush, "ArrayWithDouble"); +test(createInt32WithNewArray, "ArrayWithInt32"); +test(createInt32WithConstantLiteral, "CopyOnWriteArrayWithInt32"); +test(createEmpty, "ArrayWithUndecided"); diff --git a/JSTests/stress/array-should-not-be-converted-to-array-storage-by-frequent-frozen-arrays.js b/JSTests/stress/array-should-not-be-converted-to-array-storage-by-frequent-frozen-arrays.js new file mode 100644 index 0000000000000..cfcbc14078178 --- /dev/null +++ b/JSTests/stress/array-should-not-be-converted-to-array-storage-by-frequent-frozen-arrays.js @@ -0,0 +1,92 @@ +//@ $skipModes << :lockdown + +function shouldBe(actual, expected) { + if (actual !== expected) + throw new Error(`expected ${expected} but got ${actual}`); +} + +function create(length) { + let array = new Array(length); + for (let i = 0; i < length; ++i) + array[i] = { i }; + return array; +} +noInline(create); + +function createFrozen() { + return Object.freeze([{ i: 0 }, { i: 1 }, { i: 2 }]); +} +noInline(createFrozen); + +function isObject(value) { + return typeof value === "object"; +} + +function every(array) { + return array.every(isObject); +} + +function some(array) { + return array.some((value) => value.i === -1); +} + +function map(array) { + return array.map(isObject).length; +} + +function filter(array) { + return array.filter(isObject).length; +} + +function forEach(array) { + let count = 0; + array.forEach(() => { ++count; }); + return count; +} + +function at(array) { + return array.at(0).i; +} + +function indexed(array) { + let sum = 0; + for (let i = 0; i < array.length; ++i) + sum += array[i].i; + return sum; +} + +function forOf(array) { + let sum = 0; + for (let value of array) + sum += value.i; + return sum; +} + +function destructure(array) { + let [first, second] = array; + return first.i + second.i; +} + +function store(array) { + for (let i = 0; i < array.length; ++i) + array[i] = array[0]; +} + +function push(array) { + try { + array.push(array[0]); + } catch { } +} + +function test(func) { + for (let i = 0; i < testLoopCount; ++i) { + let array = create(8); + func(array); + shouldBe($vm.indexingMode(array), "ArrayWithContiguous"); + if (i % 2) + func(createFrozen()); + } +} + +for (let func of [every, some, map, filter, forEach, at, indexed, forOf, destructure, store, push]) + test(func); diff --git a/JSTests/stress/array-should-not-be-converted-to-array-storage-by-frozen-array.js b/JSTests/stress/array-should-not-be-converted-to-array-storage-by-frozen-array.js new file mode 100644 index 0000000000000..d95a8db49b67e --- /dev/null +++ b/JSTests/stress/array-should-not-be-converted-to-array-storage-by-frozen-array.js @@ -0,0 +1,92 @@ +//@ $skipModes << :lockdown + +function shouldBe(actual, expected) { + if (actual !== expected) + throw new Error(`expected ${expected} but got ${actual}`); +} + +function create(length) { + let array = new Array(length); + for (let i = 0; i < length; ++i) + array[i] = { i }; + return array; +} +noInline(create); + +function createFrozen() { + return Object.freeze([{ i: 0 }, { i: 1 }, { i: 2 }]); +} +noInline(createFrozen); + +function isObject(value) { + return typeof value === "object"; +} + +function every(array) { + return array.every(isObject); +} + +function some(array) { + return array.some((value) => value.i === -1); +} + +function map(array) { + return array.map(isObject).length; +} + +function filter(array) { + return array.filter(isObject).length; +} + +function forEach(array) { + let count = 0; + array.forEach(() => { ++count; }); + return count; +} + +function at(array) { + return array.at(0).i; +} + +function indexed(array) { + let sum = 0; + for (let i = 0; i < array.length; ++i) + sum += array[i].i; + return sum; +} + +function forOf(array) { + let sum = 0; + for (let value of array) + sum += value.i; + return sum; +} + +function destructure(array) { + let [first, second] = array; + return first.i + second.i; +} + +function store(array) { + for (let i = 0; i < array.length; ++i) + array[i] = array[0]; +} + +function push(array) { + try { + array.push(array[0]); + } catch { } +} + +function test(func) { + for (let i = 0; i < testLoopCount; ++i) { + let array = create(8); + func(array); + shouldBe($vm.indexingMode(array), "ArrayWithContiguous"); + if (!(i % 100)) + func(createFrozen()); + } +} + +for (let func of [every, some, map, filter, forEach, at, indexed, forOf, destructure, store, push]) + test(func); diff --git a/JSTests/stress/array-without-elements-should-not-be-converted-to-array-storage-by-frozen-array.js b/JSTests/stress/array-without-elements-should-not-be-converted-to-array-storage-by-frozen-array.js new file mode 100644 index 0000000000000..fbf24f75ead5f --- /dev/null +++ b/JSTests/stress/array-without-elements-should-not-be-converted-to-array-storage-by-frozen-array.js @@ -0,0 +1,73 @@ +//@ skip if not $jitTests +//@ runDefault("--validateOptions=true", "--useConcurrentJIT=0") + +function shouldBe(actual, expected) { + if (actual !== expected) + throw new Error(`expected ${expected} but got ${actual}`); +} + +function shouldNotBeCompiledRepeatedly(func) { + if (numberOfDFGCompiles(func) > 3) + throw new Error(`${func.name} was compiled ${numberOfDFGCompiles(func)} times`); +} + +let frozen = Object.freeze([]); + +function first(array) { + return array[0]; +} +noInline(first); + +function indexed(array) { + let count = 0; + for (let i = 0; i < array.length; ++i) { + if (array[i] === undefined) + ++count; + } + return count; +} +noInline(indexed); + +function forOf(array) { + let count = 0; + for (let value of array) + ++count; + return count; +} +noInline(forOf); + +function destructure(array) { + let [first, second] = array; + return first; +} +noInline(destructure); + +function store(array) { + array[0] = 42; +} +noInline(store); + +function push(array) { + try { + array.push(42); + } catch { } +} +noInline(push); + +function test(func, create, expected) { + for (let i = 0; i < testLoopCount; ++i) { + let array = create(); + func(array); + shouldBe($vm.indexingMode(array), expected); + if (!(i % 100)) + func(frozen); + } + shouldNotBeCompiledRepeatedly(func); +} + +test(first, () => [], "ArrayWithUndecided"); +test(indexed, () => new Array(4), "ArrayWithUndecided"); +test(forOf, () => new Array(4), "ArrayWithUndecided"); +test(destructure, () => new Array(4), "ArrayWithUndecided"); +test(store, () => [], "ArrayWithInt32"); +test(push, () => [], "ArrayWithInt32"); diff --git a/JSTests/stress/arrayify-should-not-keep-exiting-for-sparse-index.js b/JSTests/stress/arrayify-should-not-keep-exiting-for-sparse-index.js new file mode 100644 index 0000000000000..5e01b31e99909 --- /dev/null +++ b/JSTests/stress/arrayify-should-not-keep-exiting-for-sparse-index.js @@ -0,0 +1,73 @@ +//@ skip if not $jitTests +//@ runDefault("--validateOptions=true", "--useConcurrentJIT=0") + +function shouldBe(actual, expected) { + if (actual !== expected) + throw new Error(`expected ${expected} but got ${actual}`); +} + +function shouldNotBeCompiledRepeatedly(func) { + if (numberOfDFGCompiles(func) > 4) + throw new Error(`${func.name} was compiled ${numberOfDFGCompiles(func)} times`); +} + +let object = { }; +let count = testLoopCount * 10; + +function putAfterOptimized(array, index, value) { + array[index] = value; +} +noInline(putAfterOptimized); + +function putOncePerArray(array, index, value) { + array[index] = value; +} +noInline(putOncePerArray); + +function putToArrayBecomingSparse(array, index, value) { + array[index] = value; +} +noInline(putToArrayBecomingSparse); + +function read(array, index) { + return array[index]; +} +noInline(read); + +for (let i = 0; i < count; ++i) { + putAfterOptimized([1, 2, 3], 1, i); + putAfterOptimized([object, object], 1, object); +} +for (let i = 0; i < count; ++i) { + let array = []; + putAfterOptimized(array, 100008 + i, 3); + shouldBe(array[100008 + i], 3); + shouldBe(array.length, 100009 + i); +} +shouldNotBeCompiledRepeatedly(putAfterOptimized); + +for (let i = 0; i < count; ++i) { + let array = []; + putOncePerArray(array, 100008 + i, 3); + shouldBe(array[100008 + i], 3); + shouldBe(array.length, 100009 + i); +} +shouldNotBeCompiledRepeatedly(putOncePerArray); + +for (let i = 0; i < count; ++i) { + let array = []; + putToArrayBecomingSparse(array, 0, 1); + putToArrayBecomingSparse(array, 1, 2); + putToArrayBecomingSparse(array, 100008, 3); + putToArrayBecomingSparse(array, 200011, 4); + shouldBe(array[0] + array[1] + array[100008] + array[200011], 10); + shouldBe(array.length, 200012); +} +shouldNotBeCompiledRepeatedly(putToArrayBecomingSparse); + +let contiguous = [object, object, object]; +for (let i = 0; i < count; ++i) { + shouldBe(read(contiguous, 1), object); + shouldBe(read([1, 2, 3], (i % 10) ? 1 : 200000), (i % 10) ? 2 : undefined); +} +shouldNotBeCompiledRepeatedly(read); diff --git a/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.cpp b/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.cpp index 6e01ad9105be0..804e4227d05b0 100644 --- a/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.cpp +++ b/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.cpp @@ -56,7 +56,8 @@ void ArrayAllocationProfile::updateProfile() Storage storage = std::exchange(m_storage, Storage(nullptr, m_storage.type())); JSArray* lastArray = storage.pointer(); IndexingTypeAndVectorLength current = storage.type(); - if (!lastArray) + // An array that has become ArrayStorage does not tell us which type it had, so we learn nothing from it. + if (!lastArray || hasArrayStorage(lastArray->indexingType())) return; if (Options::useArrayAllocationProfiling()) [[likely]] { // The basic model here is that we will upgrade ourselves to whatever the CoW version of lastArray is except ArrayStorage since we don't have CoW ArrayStorage. diff --git a/Source/JavaScriptCore/bytecode/ArrayProfile.cpp b/Source/JavaScriptCore/bytecode/ArrayProfile.cpp index b2ad529aee3e7..ee024ccc19b6f 100644 --- a/Source/JavaScriptCore/bytecode/ArrayProfile.cpp +++ b/Source/JavaScriptCore/bytecode/ArrayProfile.cpp @@ -127,8 +127,12 @@ void ArrayProfile::computeUpdatedPrediction(CodeBlock* codeBlock) { if (auto structureID = std::exchange(m_lastSeenStructureID, StructureID())) computeUpdatedPrediction(codeBlock, structureID.decode()); - if (auto structureID = std::exchange(m_speculationFailureStructureID, StructureID())) - computeUpdatedPrediction(codeBlock, structureID.decode()); + if (auto structureID = std::exchange(m_speculationFailureStructureID, StructureID())) { + Structure* structure = structureID.decode(); + if (hasArrayStorage(structure->indexingType())) + m_arrayProfileFlags.add(ArrayProfileFlag::SpeculationFailedOnArrayStorage); + computeUpdatedPrediction(codeBlock, structure); + } } void ArrayProfile::computeUpdatedPrediction(CodeBlock* codeBlock, Structure* lastSeenStructure) diff --git a/Source/JavaScriptCore/bytecode/ArrayProfile.h b/Source/JavaScriptCore/bytecode/ArrayProfile.h index 3a3036a0f8efb..5053e17b158ef 100644 --- a/Source/JavaScriptCore/bytecode/ArrayProfile.h +++ b/Source/JavaScriptCore/bytecode/ArrayProfile.h @@ -209,6 +209,7 @@ enum class ArrayProfileFlag : uint32_t { MayBeResizableOrGrowableSharedTypedArray = 1 << 5, DidPerformFirstRunPruning = 1 << 6, MayBeRegExpMatchesArray = 1 << 7, + SpeculationFailedOnArrayStorage = 1 << 8, }; class ArrayProfile { @@ -249,6 +250,7 @@ class ArrayProfile { void computeUpdatedPrediction(CodeBlock*, Structure* lastSeenStructure); void observeArrayMode(ArrayModes mode) { m_observedArrayModes |= mode; } + void removeObservedArrayModes(ArrayModes modes) { m_observedArrayModes &= ~modes; } void NODELETE observeIndexedRead(JSCell*, unsigned index); ArrayModes observedArrayModes() const { return m_observedArrayModes; } @@ -260,6 +262,7 @@ class ArrayProfile { bool usesOriginalArrayStructures() const { return !m_arrayProfileFlags.contains(ArrayProfileFlag::UsesNonOriginalArrayStructures); } bool mayBeRegExpMatchesArray() const { return m_arrayProfileFlags.contains(ArrayProfileFlag::MayBeRegExpMatchesArray); } + bool speculationFailedOnArrayStorage() const { return m_arrayProfileFlags.contains(ArrayProfileFlag::SpeculationFailedOnArrayStorage); } UTF8CString briefDescription(CodeBlock*); UTF8CString briefDescriptionWithoutUpdating(); diff --git a/Source/JavaScriptCore/bytecode/ExitKind.h b/Source/JavaScriptCore/bytecode/ExitKind.h index 85a89a61daec3..e503180d7d09b 100644 --- a/Source/JavaScriptCore/bytecode/ExitKind.h +++ b/Source/JavaScriptCore/bytecode/ExitKind.h @@ -62,6 +62,7 @@ enum ExitKind : uint8_t { GenericUnwind, // We exited because we arrived at this OSR exit from genericUnwind. BigInt32Overflow, // We exited because of an BigInt32 overflow. UnexpectedResizableArrayBufferView, // We exited because we made an incorrect assumption about what type of ArrayBufferView we would see. + SparseIndex, // We exited because we encountered a sparse index in a place we didn't want to see it. }; bool NODELETE exitKindMayJettison(ExitKind); diff --git a/Source/JavaScriptCore/dfg/DFGArrayMode.cpp b/Source/JavaScriptCore/dfg/DFGArrayMode.cpp index 0601017db4629..4b8c6d60b8263 100644 --- a/Source/JavaScriptCore/dfg/DFGArrayMode.cpp +++ b/Source/JavaScriptCore/dfg/DFGArrayMode.cpp @@ -159,6 +159,21 @@ ArrayMode ArrayMode::fromObserved(ArrayProfile profile, Array::Action action, bo if ((observed & asArrayModesIgnoringTypedArrays(NonArray)) && profile.mayInterceptIndexedAccesses()) return ArrayMode(Array::SelectUsingPredictions).withSpeculationFromProfile(profile, makeSafe); + + // We avoid converting arrays to ArrayStorage since it misses most of the array fast paths. + constexpr ArrayModes arrayStorageModes = asArrayModesIgnoringTypedArrays(NonArrayWithArrayStorage) | asArrayModesIgnoringTypedArrays(ArrayWithArrayStorage); + if (observed & arrayStorageModes) { + bool hasSeenOtherShapes = arrayModesIncludeIgnoringTypedArrays(observed, UndecidedShape) || arrayModesIncludeIgnoringTypedArrays(observed, Int32Shape) || arrayModesIncludeIgnoringTypedArrays(observed, DoubleShape) || arrayModesIncludeIgnoringTypedArrays(observed, ContiguousShape); + bool hasSeenSlowPutArrayStorage = arrayModesIncludeIgnoringTypedArrays(observed, SlowPutArrayStorageShape); + if (hasSeenOtherShapes && !hasSeenSlowPutArrayStorage) { + // FIXME: Support ArrayStorage in MultiGetByVal and MultiPutByVal. + if (profile.speculationFailedOnArrayStorage()) + return ArrayMode(Array::Generic, nonArray, Array::AsIs, action).withProfile(profile, makeSafe); + profile.removeObservedArrayModes(arrayStorageModes); + ASSERT(!(profile.observedArrayModes() & arrayStorageModes)); + return fromObserved(profile, action, makeSafe); + } + } Array::Type type; Array::Class arrayClass; @@ -214,8 +229,8 @@ ArrayMode ArrayMode::refine( if (!isInt32Speculation(index) && !mayBeLargeTypedArray()) return ArrayMode(Array::Generic, action()); - // If we had exited because of an exotic object behavior, then don't try to specialize. - if (graph.hasExitSite(node->origin.semantic, ExoticObjectMode)) + // If we had exited because of an exotic object behavior or a sparse index, then don't try to specialize. + if (graph.hasExitSite(node->origin.semantic, ExoticObjectMode) || graph.hasExitSite(node->origin.semantic, SparseIndex)) return ArrayMode(Array::Generic, action()); if (isInBounds() && graph.hasExitSite(node->origin.semantic, LoadFromHole)) diff --git a/Source/JavaScriptCore/dfg/DFGArrayifySlowPathGenerator.h b/Source/JavaScriptCore/dfg/DFGArrayifySlowPathGenerator.h index c510e2a3a6095..8ce8eefda5c8f 100644 --- a/Source/JavaScriptCore/dfg/DFGArrayifySlowPathGenerator.h +++ b/Source/JavaScriptCore/dfg/DFGArrayifySlowPathGenerator.h @@ -60,7 +60,7 @@ class ArrayifySlowPathGenerator final : public JumpingSlowPathGeneratorspeculationCheck(Uncountable, JSValueSource(), nullptr); + m_badPropertyJump = jit->speculationCheck(SparseIndex, JSValueSource(), nullptr); break; default: break; diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp index b39482ac49f06..cd60300e9696b 100644 --- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp +++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp @@ -12826,9 +12826,15 @@ void ByteCodeParser::handleIteratorNext(const JSInstruction* currentInstruction, Node* isDone = addToGraph(CompareStrictEq, index, doneIndex); Node* iteratedObject = getIteratedObject(); - Node* butterfly = addToGraph(GetButterfly, iteratedObject); - Node* length = addToGraph(GetArrayLength, OpInfo(arrayMode.asWord()), Edge(iteratedObject), Edge(butterfly, KnownStorageUse)); - // GetArrayLength is pessimized prior to fixup. + Node* length = nullptr; + if (arrayMode.type() == Array::Generic) { + auto* data = m_graph.m_getByIdData.add(GetByIdData { CacheableIdentifier::createFromImmortalIdentifier(m_vm->propertyNames->length.impl()), CacheType::GetByIdSelf }); + length = addToGraph(GetById, OpInfo(data), OpInfo(SpecInt32Only), iteratedObject); + } else { + Node* butterfly = addToGraph(GetButterfly, iteratedObject); + length = addToGraph(GetArrayLength, OpInfo(arrayMode.asWord()), Edge(iteratedObject), Edge(butterfly, KnownStorageUse)); + } + // GetById clobbers the world, and GetArrayLength is pessimized prior to fixup. emitExitOK(); Node* isOutOfBounds = addToGraph(CompareGreaterEq, Edge(index, Int32Use), Edge(length, Int32Use)); diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp index aaa7367593f7e..76baa640a0342 100644 --- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp +++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp @@ -4434,7 +4434,7 @@ class LowerDFGToB3 { case Array::Double: case Array::Contiguous: speculate( - Uncountable, noValue(), nullptr, + SparseIndex, noValue(), nullptr, m_out.aboveOrEqual(property, m_out.constInt32(MIN_SPARSE_ARRAY_INDEX))); break; default: From 4263129d9ba1a9a1e1a32e220e31ed383e9475c9 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Tue, 13 Oct 2026 11:05:05 +0000 Subject: [PATCH 2/2] [JSC] Arrayify should not exit for a large index inside the vector Arrayify's slow path exits when the index is MIN_SPARSE_ARRAY_INDEX or more, without looking at the array. A store or a read at such an index of a dense array of that size is not sparse: the index is inside the vector, and the array only needs its conversion. That exit is now SparseIndex, which is counted, and ArrayMode::refine uses Array::Generic for the site from the next compilation on. So a site that converts large dense arrays lost its fast path for good. Stores in a loop at such a site took 1.15 ns each, 0.55 ns before the exit was counted. The slow path now exits with SparseIndex only when the object has no indexed storage or the index is at or beyond its vector length. An index inside the vector goes on to the conversion and stays in optimized code. * JSTests/stress/arrayify-should-not-exit-for-large-index-inside-vector.js: Added. * Source/JavaScriptCore/dfg/DFGArrayifySlowPathGenerator.h: * Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp: --- ...-not-exit-for-large-index-inside-vector.js | 70 +++++++++++++++++++ .../dfg/DFGArrayifySlowPathGenerator.h | 15 ++-- Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp | 22 +++++- 3 files changed, 101 insertions(+), 6 deletions(-) create mode 100644 JSTests/stress/arrayify-should-not-exit-for-large-index-inside-vector.js diff --git a/JSTests/stress/arrayify-should-not-exit-for-large-index-inside-vector.js b/JSTests/stress/arrayify-should-not-exit-for-large-index-inside-vector.js new file mode 100644 index 0000000000000..3cb2fb88ac272 --- /dev/null +++ b/JSTests/stress/arrayify-should-not-exit-for-large-index-inside-vector.js @@ -0,0 +1,70 @@ +//@ skip if not $jitTests +//@ runDefault("--validateOptions=true", "--useConcurrentJIT=0") +//@ runDefault("--validateOptions=true", "--useConcurrentJIT=0", "--useFTLJIT=0") + +function shouldBe(actual, expected) { + if (actual !== expected) + throw new Error(`expected ${expected} but got ${actual}`); +} + +let object = { }; +let count = testLoopCount * 10; +let length = 100016; +let index = 100008; + +let int32Array = []; +for (let i = 0; i < length; ++i) + int32Array.push(i); + +function makeInt32Array() { + return int32Array.slice(); +} + +function putObject(array, index, value) { + array[index] = value; +} +noInline(putObject); + +function putDouble(array, index, value) { + array[index] = value; +} +noInline(putDouble); + +function read(array, index) { + return array[index]; +} +noInline(read); + +for (let i = 0; i < count; ++i) { + putObject([1, 2, 3], 1, i); + putObject([object, object], 1, object); + putDouble([1, 2, 3], 1, i); + putDouble([1.5, 2.5], 1, 3.5); + shouldBe(read([1, 2, 3], 1), 2); + shouldBe(read([object, object], 1), object); +} + +// These indices are large, but they are inside the vector. So the arrays are converted in the optimized code. +let readCompiles = numberOfDFGCompiles(read); +let array = makeInt32Array(); +for (let i = 0; i < count; ++i) + shouldBe(read(array, index), index); +shouldBe(array.length, length); +shouldBe(numberOfDFGCompiles(read), readCompiles); + +for (let i = 0; i < 8; ++i) { + array = makeInt32Array(); + shouldBe($vm.indexingMode(array), "ArrayWithInt32"); + putObject(array, index, object); + shouldBe($vm.indexingMode(array), "ArrayWithContiguous"); + shouldBe(array[index], object); + shouldBe(array[index - 1], index - 1); + shouldBe(array.length, length); + + array = makeInt32Array(); + putDouble(array, index, 0.5); + shouldBe($vm.indexingMode(array), "ArrayWithDouble"); + shouldBe(array[index], 0.5); + shouldBe(array[index - 1], index - 1); + shouldBe(array.length, length); +} diff --git a/Source/JavaScriptCore/dfg/DFGArrayifySlowPathGenerator.h b/Source/JavaScriptCore/dfg/DFGArrayifySlowPathGenerator.h index 8ce8eefda5c8f..d69df57af38f3 100644 --- a/Source/JavaScriptCore/dfg/DFGArrayifySlowPathGenerator.h +++ b/Source/JavaScriptCore/dfg/DFGArrayifySlowPathGenerator.h @@ -80,11 +80,18 @@ class ArrayifySlowPathGenerator final : public JumpingSlowPathGeneratorbranch32( - MacroAssembler::AboveOrEqual, m_propertyGPR, - MacroAssembler::TrustedImm32(MIN_SPARSE_ARRAY_INDEX))); + case Array::Contiguous: { + // An index inside the vector is not sparse, however large it is. + MacroAssembler::JumpList sparseIndex; + auto smallIndex = jit->branch32(MacroAssembler::Below, m_propertyGPR, MacroAssembler::TrustedImm32(MIN_SPARSE_ARRAY_INDEX)); + jit->load8(MacroAssembler::Address(m_baseGPR, JSCell::indexingTypeAndMiscOffset()), m_tempGPR); + sparseIndex.append(jit->branchTest32(MacroAssembler::Zero, m_tempGPR, MacroAssembler::TrustedImm32(IndexingShapeMask))); + jit->loadPtr(MacroAssembler::Address(m_baseGPR, JSObject::butterflyOffset()), m_tempGPR); + sparseIndex.append(jit->branch32(MacroAssembler::AboveOrEqual, m_propertyGPR, MacroAssembler::Address(m_tempGPR, Butterfly::offsetOfVectorLength()))); + smallIndex.link(jit); + m_badPropertyJump.fill(jit, sparseIndex); break; + } default: break; } diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp index 76baa640a0342..4c8a9e3571c61 100644 --- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp +++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp @@ -4432,11 +4432,29 @@ class LowerDFGToB3 { switch (m_node->arrayMode().type()) { case Array::Int32: case Array::Double: - case Array::Contiguous: + case Array::Contiguous: { + // An index inside the vector is not sparse, however large it is. + LBasicBlock largeIndex = m_out.newBlock(); + LBasicBlock convert = m_out.newBlock(); + + m_out.branch( + m_out.aboveOrEqual(property, m_out.constInt32(MIN_SPARSE_ARRAY_INDEX)), + rarely(largeIndex), usually(convert)); + + m_out.appendTo(largeIndex, convert); + LValue indexingType = m_out.load8ZeroExt32(cell, m_heaps.JSCell_indexingTypeAndMisc); speculate( SparseIndex, noValue(), nullptr, - m_out.aboveOrEqual(property, m_out.constInt32(MIN_SPARSE_ARRAY_INDEX))); + m_out.testIsZero32(indexingType, m_out.constInt32(IndexingShapeMask))); + LValue butterfly = m_out.loadPtr(cell, m_heaps.JSObject_butterfly); + speculate( + SparseIndex, noValue(), nullptr, + m_out.aboveOrEqual(property, m_out.load32NonNegative(butterfly, m_heaps.Butterfly_vectorLength))); + m_out.jump(convert); + + m_out.appendTo(convert, continuation); break; + } default: break; }