Skip to content

Commit 1ed1e4a

Browse files
ConstellationConstellation
committed
[JSC] Make JSMap and JSSet construction more simple and efficient
https://bugs.webkit.org/show_bug.cgi?id=243557 rdar://98068082 Reviewed by Mark Lam and Saam Barati. This patch makes the initial buffer of JSMap / JSSet nullptr so that we can make allocation of them simpler and efficient for non-using case. It cleans up many code in module loader etc. And it paves the way to allocating them from DFG and FTL efficiently. It also cleans up SerializedScriptValue implementation. * JSTests/stress/map-clear-get.js: Added. (shouldBe): (test): * JSTests/stress/set-clear-has.js: Added. (shouldBe): (set clear): (set shouldBe): (set new): * Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::compile): * Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq): * Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp: (JSC::AbstractModuleRecord::finishCreation): * Source/JavaScriptCore/runtime/HashMapImpl.h: (JSC::HashMapBuffer::tryCreate): (JSC::HashMapImpl::HashMapImpl): (JSC::HashMapBuffer::create): Deleted. (JSC::HashMapImpl::shouldRehashAfterAdd const): Deleted. * Source/JavaScriptCore/runtime/HashMapImplInlines.h: (JSC::shouldShrink): (JSC::shouldRehash): (JSC::nextCapacity): (JSC::HashMapImpl<HashMapBucketType>::finishCreation): (JSC::HashMapImpl<HashMapBucketType>::add): (JSC::HashMapImpl<HashMapBucketType>::addNormalized): (JSC::HashMapImpl<HashMapBucketType>::remove): (JSC::HashMapImpl<HashMapBucketType>::clear): (JSC::HashMapImpl<HashMapBucketType>::setUpHeadAndTail): (JSC::HashMapImpl<HashMapBucketType>::addNormalizedNonExistingForCloning): (JSC::HashMapImpl<HashMapBucketType>::addNormalizedNonExistingForCloningInternal): (JSC::HashMapImpl<HashMapBucketType>::addNormalizedInternal): (JSC::HashMapImpl<HashMapBucketType>::findBucketAlreadyHashedAndNormalized): (JSC::HashMapImpl<HashMapBucketType>::rehash): (JSC::HashMapImpl<HashMapBucketType>::makeAndSetNewBuffer): (JSC::HashMapImpl<HashMapBucketType>::assertBufferIsEmpty): (JSC::shouldRehashAfterAdd): Deleted. (JSC::HashMapImpl<HashMapBucketType>::assertBufferIsEmpty const): Deleted. * Source/JavaScriptCore/runtime/JSMap.h: * Source/JavaScriptCore/runtime/JSModuleLoader.cpp: (JSC::JSModuleLoader::finishCreation): * Source/JavaScriptCore/runtime/JSSet.h: * Source/JavaScriptCore/runtime/MapConstructor.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/MapPrototype.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/SetConstructor.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/SetPrototype.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/WeakMapImplInlines.h: (JSC::WeakMapImpl<WeakMapBucket>::shouldRehashAfterAdd const): * Source/WebCore/bindings/js/JSDOMMapLike.cpp: (WebCore::getBackingMap): * Source/WebCore/bindings/js/JSDOMSetLike.cpp: (WebCore::getBackingSet): * Source/WebCore/bindings/js/SerializedScriptValue.cpp: (WebCore::CloneDeserializer::deserialize): Canonical link: https://commits.webkit.org/253133@main
1 parent 5374055 commit 1ed1e4a

18 files changed

Lines changed: 170 additions & 146 deletions

‎JSTests/stress/map-clear-get.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
function shouldBe(actual, expected) {
2+
if (actual !== expected)
3+
throw new Error('bad value: ' + actual);
4+
}
5+
6+
function test(map, key)
7+
{
8+
return map.get(key);
9+
}
10+
noInline(test);
11+
12+
var map = new Map();
13+
for (var i = 0; i < 1e4; ++i) {
14+
map.clear();
15+
shouldBe(test(map, {}), undefined);
16+
shouldBe(test(map, {t:42}), undefined);
17+
}

‎JSTests/stress/set-clear-has.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
function shouldBe(actual, expected) {
2+
if (actual !== expected)
3+
throw new Error('bad value: ' + actual);
4+
}
5+
6+
function test(set, key)
7+
{
8+
return set.has(key);
9+
}
10+
noInline(test);
11+
12+
var set = new Set();
13+
for (var i = 0; i < 1e4; ++i) {
14+
set.clear();
15+
shouldBe(test(set, {}), false);
16+
shouldBe(test(set, {t:42}), false);
17+
}

‎Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5000,8 +5000,11 @@ void SpeculativeJIT::compile(Node* node)
50005000
if (node->child2().useKind() != UntypedUse)
50015001
speculate(node, node->child2());
50025002

5003-
m_jit.load32(MacroAssembler::Address(mapGPR, HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::offsetOfCapacity()), maskGPR);
5003+
CCallHelpers::JumpList notPresentInTable;
5004+
50045005
m_jit.loadPtr(MacroAssembler::Address(mapGPR, HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::offsetOfBuffer()), bufferGPR);
5006+
notPresentInTable.append(m_jit.branchTestPtr(CCallHelpers::Zero, bufferGPR));
5007+
m_jit.load32(MacroAssembler::Address(mapGPR, HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::offsetOfCapacity()), maskGPR);
50055008
m_jit.sub32(TrustedImm32(1), maskGPR);
50065009
m_jit.move(hashGPR, indexGPR);
50075010

@@ -5013,8 +5016,9 @@ void SpeculativeJIT::compile(Node* node)
50135016
m_jit.and32(maskGPR, indexGPR);
50145017
m_jit.loadPtr(MacroAssembler::BaseIndex(bufferGPR, indexGPR, MacroAssembler::TimesEight), bucketGPR);
50155018
m_jit.move(bucketGPR, resultGPR);
5016-
auto notPresentInTable = m_jit.branchPtr(MacroAssembler::Equal,
5017-
bucketGPR, TrustedImmPtr(bitwise_cast<size_t>(HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::emptyValue())));
5019+
5020+
notPresentInTable.append(m_jit.branchPtr(MacroAssembler::Equal,
5021+
bucketGPR, TrustedImmPtr(bitwise_cast<size_t>(HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::emptyValue()))));
50185022
loopAround.append(m_jit.branchPtr(MacroAssembler::Equal,
50195023
bucketGPR, TrustedImmPtr(bitwise_cast<size_t>(HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::deletedValue()))));
50205024

‎Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12340,6 +12340,7 @@ IGNORE_CLANG_WARNINGS_END
1234012340
void compileGetMapBucket()
1234112341
{
1234212342
JSGlobalObject* globalObject = m_graph.globalObjectFor(m_origin.semantic);
12343+
LBasicBlock indexSetUp = m_out.newBlock();
1234312344
LBasicBlock loopStart = m_out.newBlock();
1234412345
LBasicBlock loopAround = m_out.newBlock();
1234512346
LBasicBlock slowPath = m_out.newBlock();
@@ -12348,8 +12349,6 @@ IGNORE_CLANG_WARNINGS_END
1234812349
LBasicBlock notDeletedValue = m_out.newBlock();
1234912350
LBasicBlock continuation = m_out.newBlock();
1235012351

12351-
LBasicBlock lastNext = m_out.insertNewBlocksBefore(loopStart);
12352-
1235312352
LValue map;
1235412353
if (m_node->child1().useKind() == MapObjectUse)
1235512354
map = lowMapObject(m_node->child1());
@@ -12365,8 +12364,11 @@ IGNORE_CLANG_WARNINGS_END
1236512364
LValue hash = lowInt32(m_node->child3());
1236612365

1236712366
LValue buffer = m_out.loadPtr(map, m_heaps.HashMapImpl_buffer);
12368-
LValue mask = m_out.sub(m_out.load32(map, m_heaps.HashMapImpl_capacity), m_out.int32One);
1236912367

12368+
m_out.branch(m_out.isNull(buffer), unsure(notPresentInTable), unsure(indexSetUp));
12369+
12370+
LBasicBlock lastNext = m_out.appendTo(indexSetUp, loopStart);
12371+
LValue mask = m_out.sub(m_out.load32(map, m_heaps.HashMapImpl_capacity), m_out.int32One);
1237012372
ValueFromBlock indexStart = m_out.anchor(hash);
1237112373
m_out.jump(loopStart);
1237212374

‎Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@ AbstractModuleRecord::AbstractModuleRecord(VM& vm, Structure* structure, const I
5151

5252
void AbstractModuleRecord::finishCreation(JSGlobalObject* globalObject, VM& vm)
5353
{
54-
DeferTerminationForAWhile deferScope(vm);
55-
auto scope = DECLARE_CATCH_SCOPE(vm);
56-
5754
Base::finishCreation(vm);
5855
ASSERT(inherits(info()));
5956

@@ -62,8 +59,7 @@ void AbstractModuleRecord::finishCreation(JSGlobalObject* globalObject, VM& vm)
6259
for (unsigned index = 0; index < values.size(); ++index)
6360
Base::internalField(index).set(vm, this, values[index]);
6461

65-
JSMap* map = JSMap::create(globalObject, vm, globalObject->mapStructure());
66-
scope.releaseAssertNoException();
62+
JSMap* map = JSMap::create(vm, globalObject->mapStructure());
6763
m_dependenciesMap.set(vm, this, map);
6864
putDirect(vm, Identifier::fromString(vm, "dependenciesMap"_s), m_dependenciesMap.get());
6965
}

‎Source/JavaScriptCore/runtime/HashMapImpl.h‎

Lines changed: 14 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ class HashMapBuffer {
209209
return bitwise_cast<BucketType**>(this);
210210
}
211211

212-
static HashMapBuffer* create(JSGlobalObject* globalObject, VM& vm, JSCell*, uint32_t capacity)
212+
static HashMapBuffer* tryCreate(JSGlobalObject* globalObject, VM& vm, uint32_t capacity)
213213
{
214214
auto scope = DECLARE_THROW_SCOPE(vm);
215215
size_t allocationSize = HashMapBuffer::allocationSize(capacity);
@@ -239,7 +239,7 @@ ALWAYS_INLINE uint32_t wangsInt64Hash(uint64_t key);
239239
ALWAYS_INLINE uint32_t jsMapHash(JSBigInt*);
240240
ALWAYS_INLINE uint32_t jsMapHash(JSGlobalObject*, VM&, JSValue);
241241
ALWAYS_INLINE uint32_t shouldShrink(uint32_t capacity, uint32_t keyCount);
242-
ALWAYS_INLINE uint32_t shouldRehashAfterAdd(uint32_t capacity, uint32_t keyCount, uint32_t deleteCount);
242+
ALWAYS_INLINE uint32_t shouldRehash(uint32_t capacity, uint32_t keyCount, uint32_t deleteCount);
243243
ALWAYS_INLINE uint32_t nextCapacity(uint32_t capacity, uint32_t keyCount);
244244

245245
template <typename HashMapBucketType>
@@ -256,28 +256,15 @@ class HashMapImpl : public JSNonFinalObject {
256256

257257
HashMapImpl(VM& vm, Structure* structure)
258258
: Base(vm, structure)
259-
, m_keyCount(0)
260-
, m_deleteCount(0)
261-
, m_capacity(4)
262259
{
263260
}
264261

265-
HashMapImpl(VM& vm, Structure* structure, uint32_t sizeHint)
266-
: Base(vm, structure)
267-
, m_keyCount(0)
268-
, m_deleteCount(0)
269-
{
270-
uint32_t capacity = (Checked<uint32_t>(sizeHint) * 2) + 1;
271-
capacity = std::max<uint32_t>(WTF::roundUpToPowerOfTwo(capacity), 4U);
272-
m_capacity = capacity;
273-
}
274-
275262
ALWAYS_INLINE HashMapBucketType** buffer() const
276263
{
277264
return m_buffer->buffer();
278265
}
279266

280-
void finishCreation(JSGlobalObject*, VM&);
267+
void finishCreation(VM&);
281268
void finishCreation(JSGlobalObject*, VM&, HashMapImpl* base);
282269

283270
static HashMapBucketType* emptyValue()
@@ -320,7 +307,7 @@ class HashMapImpl : public JSNonFinalObject {
320307
return m_keyCount;
321308
}
322309

323-
ALWAYS_INLINE void clear(JSGlobalObject*);
310+
ALWAYS_INLINE void clear(VM&);
324311

325312
ALWAYS_INLINE size_t bufferSizeInBytes() const
326313
{
@@ -355,42 +342,39 @@ class HashMapImpl : public JSNonFinalObject {
355342
}
356343

357344
private:
358-
ALWAYS_INLINE uint32_t shouldRehashAfterAdd() const
359-
{
360-
return JSC::shouldRehashAfterAdd(m_capacity, m_keyCount, m_deleteCount);
361-
}
362-
363345
ALWAYS_INLINE uint32_t shouldShrink() const
364346
{
365347
return JSC::shouldShrink(m_capacity, m_keyCount);
366348
}
367349

368-
ALWAYS_INLINE void setUpHeadAndTail(JSGlobalObject*, VM&);
350+
ALWAYS_INLINE void setUpHeadAndTail(VM&);
369351

370352
ALWAYS_INLINE void addNormalizedNonExistingForCloning(JSGlobalObject*, JSValue key, JSValue = JSValue());
353+
ALWAYS_INLINE HashMapBucketType* addNormalizedNonExistingForCloningInternal(JSGlobalObject*, JSValue key, JSValue, uint32_t hash);
371354

372355
template<typename CanUseBucket>
373356
ALWAYS_INLINE void addNormalizedInternal(JSGlobalObject*, JSValue key, JSValue, const CanUseBucket&);
374357

375358
template<typename CanUseBucket>
376-
ALWAYS_INLINE HashMapBucketType* addNormalizedInternal(VM&, JSValue key, JSValue, uint32_t hash, const CanUseBucket&);
359+
ALWAYS_INLINE HashMapBucketType* addNormalizedInternal(JSGlobalObject*, JSValue key, JSValue, uint32_t hash, const CanUseBucket&);
377360

378361
ALWAYS_INLINE HashMapBucketType** findBucketAlreadyHashedAndNormalized(JSGlobalObject*, JSValue key, uint32_t hash);
379362

380-
void rehash(JSGlobalObject*);
363+
enum class RehashMode { BeforeAddition, AfterRemoval };
364+
void rehash(JSGlobalObject*, RehashMode);
381365

382366
ALWAYS_INLINE void checkConsistency() const;
383367

384-
void makeAndSetNewBuffer(JSGlobalObject*, VM&);
368+
void makeAndSetNewBuffer(JSGlobalObject*, uint32_t newCapacity, VM&);
385369

386-
ALWAYS_INLINE void assertBufferIsEmpty() const;
370+
ALWAYS_INLINE static void assertBufferIsEmpty(HashMapBucketType**, uint32_t capacity);
387371

388372
WriteBarrier<HashMapBucketType> m_head;
389373
WriteBarrier<HashMapBucketType> m_tail;
390374
AuxiliaryBarrier<HashMapBufferType*> m_buffer;
391-
uint32_t m_keyCount;
392-
uint32_t m_deleteCount;
393-
uint32_t m_capacity;
375+
uint32_t m_keyCount { 0 };
376+
uint32_t m_deleteCount { 0 };
377+
uint32_t m_capacity { 0 };
394378
};
395379

396380
} // namespace JSC

0 commit comments

Comments
 (0)