Reland StructureID overhaul
Created attachment 450155 [details] Patch
Comment on attachment 450155 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=450155&action=review r=me > Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp:105 > + MarkedBlock* block = reinterpret_cast<MarkedBlock*>(g_jscConfig.startOfStructureHeap) + freeIndex * MarkedBlock::blockSize; > + constexpr bool writable = true; > + constexpr bool executable = false; > + OSAllocator::commit(block, MarkedBlock::blockSize, writable, executable); Let's mprotect READ | WRITE in debug build. > Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp:111 > + OSAllocator::decommit(blockPtr, MarkedBlock::blockSize); Let's mprotect NONE in debug build. > Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp:149 > +void StructureAlignedMemoryAllocator::commitBlock(void* block) > +{ > + constexpr bool writable = true; > + constexpr bool executable = false; > + OSAllocator::commit(block, MarkedBlock::blockSize, writable, executable); > +} Let's mprotect READ | WRITE in debug build. > Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp:154 > +void StructureAlignedMemoryAllocator::decommitBlock(void* block) > +{ > + OSAllocator::decommit(block, MarkedBlock::blockSize); > +} Let's mprotect NONE in debug build.
Created attachment 450407 [details] Patch for landing
Committed r288815 (246591@main): <https://commits.webkit.org/246591@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 450407 [details].
<rdar://problem/88270829>
Comment on attachment 450407 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=450407&action=review > Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp:85 > + ASSERT((g_jscConfig.startOfStructureHeap & ~structureIDMask) == g_jscConfig.startOfStructureHeap); nits: - Should be RELEASE_ASSERT - And let's also RELEASE_ASSERT that startOfStructureHeap isn't null. > Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp:95 > + ASSERT(freeIndex <= m_usedBlocks.bitCount()); This code is also quite subtle. You grow the bit vector by "not finding a bit", and having find return bitCount(). Might be worth a comment. > Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp:102 > + MarkedBlock* block = reinterpret_cast<MarkedBlock*>(g_jscConfig.startOfStructureHeap) + freeIndex * MarkedBlock::blockSize; same nit from last patch: use uint8_t* here instead of MarkedBlocked*
Comment on attachment 450407 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=450407&action=review > Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp:92 > + Locker locker(m_lock); Not sure we need a lock here, given the IsoMemoryAllocatorBase already locks. We could pass around a locker if we wanted to indicate this.