Created attachment 390953 [details] Patch

Created attachment 390954 [details] Patch

Created attachment 391014 [details] Patch

<rdar://problem/59534793>

The failing test is known that it is relying on pointer orders. So, this is flaky.

Created attachment 391023 [details] Patch

Comment on attachment 391023 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391023&action=review > Source/JavaScriptCore/ChangeLog:14 > + 2. Instead of using bloom-filter of pointers for seen-properties, we should use bloom-filter of hash-codes. You sure this is ok? I specifically didn’t do this because it made the bloom filter way less effective (IIRC, I measured rule-out rate running Speedo2 and browsing the web)

Comment on attachment 391023 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391023&action=review > Source/JavaScriptCore/runtime/Structure.h:501 > bool hasInlineStorage() const > { > - return !!m_inlineCapacity; > - } > - unsigned inlineCapacity() const > - { > - return m_inlineCapacity; > + return !!inlineCapacity(); > } nit: This can be a 1 liner too for compactness. > Source/JavaScriptCore/runtime/Structure.h:636 > +#if CPU(LITTLE_ENDIAN) > + static ptrdiff_t offsetOfInlineCapacity() > { > - return OBJECT_OFFSETOF(Structure, m_inlineCapacity); > + return OBJECT_OFFSETOF(Structure, m_propertyHashAndInlineCapacity); > } > +#endif Can you just provide the BIG_ENDIAN version as well? The MIPS folks would probably appreciate that. I'm thinking OBJECT_OFFSETOF(Structure, m_propertyHashAndInlineCapacity) + 3? > Source/JavaScriptCore/runtime/Structure.h:763 > + DEFINE_BITFIELD(unsigned, transitionPropertyAttributes, TransitionPropertyAttributes, 8, 6); Is this enough bits? Shouldn't this be 10 bits? > Source/JavaScriptCore/runtime/Structure.h:776 > static_assert(s_bitWidthOfTransitionPropertyAttributes <= sizeof(TransitionPropertyAttributes) * 8); Why <= here? Why not ==? On the other hand, if I'm correct that TransitionPropertyAttributes needs to be 10 bits, then this static_assert should stay unchanged. > Source/JavaScriptCore/runtime/Structure.h:894 > + // Should be accessed through ensurePropertyTable(). During GC, it may be set to 0 by another thread. > + // During a Heap Snapshot GC we avoid clearing the table so it is safe to use. This comment used to be associated with m_propertyTableUnsafe. Now, this association is not totally clear. I'm also not sure if the comment is stale, but at minimum, we should give it some context, like: The property table pointer should be accessed through ensurePropertyTable(). During GC, ... > Source/JavaScriptCore/runtime/Structure.h:916 > + PackedRefPtr<UniquedStringImpl> m_transitionPropertyName; This is tricky. At first, I was wondering how this saves any space. And then I realized that StructureTransitionTable is byte aligned, and only contains a PackedPtr. Can you static_assert that PackedRefPtr and StructureTransitionTable are byte aligned? I think that this is essential in order for m_bitField to get packed in with them to only use 32 bytes. > Source/JavaScriptCore/runtime/StructureInlines.h:149 > + unsigned hashCode = propertyName.uid()->symbolAwareHash(); > + ASSERT(hashCode); // StringImpl hash never gets zero. > + if ((m_seenProperties & hashCode) != hashCode) It would be good to factor this into an inline function ruleOutUnseenPropertyHash() and locate it near the addSeenPropertyHash() below. The reason for doing this is so that the 2 are co-located, and it becomes easier to see that you're implementing a bloom filter here. Right now, in this review, it's easy to see that, but it becomes less clear for someone reading this code later who does not have the benefit of reading this diff. > Source/JavaScriptCore/runtime/StructureInlines.h:499 > + unsigned hashCode = rep->existingSymbolAwareHash(); > + setPropertyHash(propertyHash() ^ hashCode); > + m_seenProperties = m_seenProperties | hashCode; Let's put this in an inline function addSeenPropertyHash() and locate it near the ruleOutUnseenPropertyHash() function above. > Source/JavaScriptCore/runtime/StructureTransitionTable.h:61 > +inline constexpr uint8_t toAttributes(NonPropertyTransition transition) > { > return static_cast<unsigned>(transition) + FirstInternalAttribute; > } How can this be correct? FirstInternalAttribute is 1 << 6 i.e. 6 bits. There are 11 NonPropertyTransitions i.e. 4 bits. Total unique bits = 10. Aren't we potentially losing the top 2 bits here?

Comment on attachment 391023 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391023&action=review r=me with other suggestions, and if there's no perf regression. >> Source/JavaScriptCore/runtime/StructureTransitionTable.h:61 >> } > > How can this be correct? FirstInternalAttribute is 1 << 6 i.e. 6 bits. There are 11 NonPropertyTransitions i.e. 4 bits. Total unique bits = 10. Aren't we potentially losing the top 2 bits here? I was wrong. Yusuke pointed out that we're computing NonPropertyTransition + 1<<6, not NonPropertyTransition << 6.

Comment on attachment 391023 [details] Patch Clearing the r+ for now. Yusuke and I have spotted some concurrency issues.

Created attachment 391443 [details] Patch

Comment on attachment 391023 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391023&action=review >> Source/JavaScriptCore/ChangeLog:14 >> + 2. Instead of using bloom-filter of pointers for seen-properties, we should use bloom-filter of hash-codes. > > You sure this is ok? I specifically didn’t do this because it made the bloom filter way less effective (IIRC, I measured rule-out rate running Speedo2 and browsing the web) For now, using pointer based filter. >> Source/JavaScriptCore/runtime/Structure.h:501 >> } > > nit: This can be a 1 liner too for compactness. Fixed. >> Source/JavaScriptCore/runtime/Structure.h:636 >> +#endif > > Can you just provide the BIG_ENDIAN version as well? The MIPS folks would probably appreciate that. I'm thinking OBJECT_OFFSETOF(Structure, m_propertyHashAndInlineCapacity) + 3? I think this is not necessary. This function is used only from JIT, and our JIT only supports little-endian environment. >> Source/JavaScriptCore/runtime/Structure.h:763 >> + DEFINE_BITFIELD(unsigned, transitionPropertyAttributes, TransitionPropertyAttributes, 8, 6); > > Is this enough bits? Shouldn't this be 10 bits? Discussed with Mark, it is enough. >> Source/JavaScriptCore/runtime/Structure.h:776 >> static_assert(s_bitWidthOfTransitionPropertyAttributes <= sizeof(TransitionPropertyAttributes) * 8); > > Why <= here? Why not ==? > > On the other hand, if I'm correct that TransitionPropertyAttributes needs to be 10 bits, then this static_assert should stay unchanged. So long as it is smaller than TransitionPropertyAttributes's size, it is OK. >> Source/JavaScriptCore/runtime/Structure.h:894 >> + // During a Heap Snapshot GC we avoid clearing the table so it is safe to use. > > This comment used to be associated with m_propertyTableUnsafe. Now, this association is not totally clear. I'm also not sure if the comment is stale, but at minimum, we should give it some context, like: > > The property table pointer should be accessed through ensurePropertyTable(). During GC, ... Fixed. >> Source/JavaScriptCore/runtime/Structure.h:916 >> + PackedRefPtr<UniquedStringImpl> m_transitionPropertyName; > > This is tricky. At first, I was wondering how this saves any space. And then I realized that StructureTransitionTable is byte aligned, and only contains a PackedPtr. Can you static_assert that PackedRefPtr and StructureTransitionTable are byte aligned? I think that this is essential in order for m_bitField to get packed in with them to only use 32 bytes. Added. >> Source/JavaScriptCore/runtime/StructureInlines.h:149 >> + if ((m_seenProperties & hashCode) != hashCode) > > It would be good to factor this into an inline function ruleOutUnseenPropertyHash() and locate it near the addSeenPropertyHash() below. The reason for doing this is so that the 2 are co-located, and it becomes easier to see that you're implementing a bloom filter here. Right now, in this review, it's easy to see that, but it becomes less clear for someone reading this code later who does not have the benefit of reading this diff. Fixed. >> Source/JavaScriptCore/runtime/StructureInlines.h:499 >> + m_seenProperties = m_seenProperties | hashCode; > > Let's put this in an inline function addSeenPropertyHash() and locate it near the ruleOutUnseenPropertyHash() function above. I've added addPropertyHashAndSeenProperty, which take hash and property, and update propertyHash and seenProperties.

Created attachment 391452 [details] Patch

Created attachment 391453 [details] Patch

Comment on attachment 391452 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391452&action=review I only some of the files for now > Source/JavaScriptCore/ChangeLog:14 > + 2. Instead of using bloom-filter of pointers for seen-properties, we should use bloom-filter of hash-codes. Please update changelog since you apparently removed that change > Source/JavaScriptCore/ChangeLog:29 > + Combining all of the above techniques and getting 16 bytes. nit: and getting => gets us > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:12937 > + m_jit.loadPtr(CCallHelpers::Address(structureGPR, Structure::offsetOfClassInfo()), scratch1GPR); This pattern to load the classInfo of a structure seems to recur often enough to justify a small helper in AssemblyHelpers.h, just to help readability. > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6094 > + LValue previousOrRareData = m_out.bitAnd(m_out.constIntPtr(Structure::previousOrRareDataMask), m_out.loadPtr(structure, m_heaps.Structure_previousOrRareData)); Structure::previousOrRareDataMask was protected by #if USE(JSVALUE64) before. Which one was correct? This is another example of a pattern which would be less error prone if it was written only once in a helper function. Also please test this patch on a 32-bit platform if you have not done so yet. The code paths are sufficiently different on 32 and 64 bits to be a likely source of bugs. > Source/JavaScriptCore/runtime/ConcurrentJSLock.h:37 > static_assert(sizeof(ConcurrentJSLock) == 1, "Regardless of status of concurrent JS flag, size of ConurrentJSLock is always one byte."); unrelated typo: Conurrent => Concurrent > Source/JavaScriptCore/runtime/Structure.cpp:367 > + Vector<std::pair<Structure*, UniquedStringImpl*>, 8> structures; I am confused by this change. 1) The name no longer match 2) More importantly I can not find any access to the second element > Source/JavaScriptCore/runtime/Structure.cpp:709 > // This must always return a property table. It can't return null. This comment is confusing. Does "This" refer to the next line (clearly wrong then) or to the overall function (In this case it would be much clearer to s/This/This function/) > Source/JavaScriptCore/runtime/Structure.cpp:1138 > // That's fine, because then the barrier will fire and we will scan this again. This comment refer to a barrier, but you modify the next line to use appendUnbarriered, is it normal? > Source/JavaScriptCore/runtime/WriteBarrier.h:251 > +using UnsafeCellPointer = uintptr_t; Where is this used? > Source/WTF/wtf/CompactPointerTuple.h:34 > // The goal of this class is folding a pointer and 1 byte value into 8 bytes in both 32bit and 64bit architectures. Please update comment since this class can now store 2 bytes + a pointer. > Source/WTF/wtf/CompactPointerTuple.h:59 > + static constexpr uint64_t encodeType(Type type) This could be a private method. > Source/WTF/wtf/CompactPointerTuple.h:63 > + static constexpr Type decodeType(uint64_t value) Ditto. > Source/WTF/wtf/text/SymbolImpl.h:44 > + unsigned hashForSymbol() const { return m_hashForSymbol >> s_flagCount; } The name of "m_hashForSymbol" is not great if it stores more than just hashForSymbol

Comment on attachment 391452 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391452&action=review >> Source/JavaScriptCore/ChangeLog:14 >> + 2. Instead of using bloom-filter of pointers for seen-properties, we should use bloom-filter of hash-codes. > > Please update changelog since you apparently removed that change Fixed. >> Source/JavaScriptCore/ChangeLog:29 >> + Combining all of the above techniques and getting 16 bytes. > > nit: and getting => gets us Fixed. >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:12937 >> + m_jit.loadPtr(CCallHelpers::Address(structureGPR, Structure::offsetOfClassInfo()), scratch1GPR); > > This pattern to load the classInfo of a structure seems to recur often enough to justify a small helper in AssemblyHelpers.h, just to help readability. Sounds nice. Fixed. >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6094 >> + LValue previousOrRareData = m_out.bitAnd(m_out.constIntPtr(Structure::previousOrRareDataMask), m_out.loadPtr(structure, m_heaps.Structure_previousOrRareData)); > > Structure::previousOrRareDataMask was protected by #if USE(JSVALUE64) before. Which one was correct? > This is another example of a pattern which would be less error prone if it was written only once in a helper function. > Also please test this patch on a 32-bit platform if you have not done so yet. The code paths are sufficiently different on 32 and 64 bits to be a likely source of bugs. FTL only supports 64bit, and all the other code in FTL strongly assumes 64bit right now. So, I think this is OK. >> Source/JavaScriptCore/runtime/ConcurrentJSLock.h:37 >> static_assert(sizeof(ConcurrentJSLock) == 1, "Regardless of status of concurrent JS flag, size of ConurrentJSLock is always one byte."); > > unrelated typo: Conurrent => Concurrent Fixed. >> Source/JavaScriptCore/runtime/Structure.cpp:367 >> + Vector<std::pair<Structure*, UniquedStringImpl*>, 8> structures; > > I am confused by this change. > 1) The name no longer match > 2) More importantly I can not find any access to the second element 1) I'll update 2) UniquedStringImpl* is used in Structure::forEachPropertyConcurrently. This is added for that function because forEachPropertyConcurrently can be called concurrently. In that case, we should collect UniquedStringImpl* when structure lock is held. For consistency, I'll use this UniquedStringImpl* in this function too. >> Source/JavaScriptCore/runtime/Structure.cpp:709 >> // This must always return a property table. It can't return null. > > This comment is confusing. Does "This" refer to the next line (clearly wrong then) or to the overall function (In this case it would be much clearer to s/This/This function/) This is "This function". Updated. >> Source/JavaScriptCore/runtime/Structure.cpp:1138 >> // That's fine, because then the barrier will fire and we will scan this again. > > This comment refer to a barrier, but you modify the next line to use appendUnbarriered, is it normal? Yes. visitor.appendUnbarriered is used when we do not have WriteBarrier<> instance for this field, (Unbarrier => no WriteBarrier<> field). But when setting propertyTableUnsafeOrNull's field, we are emitting a barrier (vm.heap.writeBarrier), so barrier exists. >> Source/JavaScriptCore/runtime/WriteBarrier.h:251 >> +using UnsafeCellPointer = uintptr_t; > > Where is this used? This was originally used, but it is no longer used, removed. >> Source/WTF/wtf/CompactPointerTuple.h:34 >> // The goal of this class is folding a pointer and 1 byte value into 8 bytes in both 32bit and 64bit architectures. > > Please update comment since this class can now store 2 bytes + a pointer. Fixed. >> Source/WTF/wtf/CompactPointerTuple.h:59 >> + static constexpr uint64_t encodeType(Type type) > > This could be a private method. Right, fixed. >> Source/WTF/wtf/CompactPointerTuple.h:63 >> + static constexpr Type decodeType(uint64_t value) > > Ditto. Fixed. >> Source/WTF/wtf/text/SymbolImpl.h:44 >> + unsigned hashForSymbol() const { return m_hashForSymbol >> s_flagCount; } > > The name of "m_hashForSymbol" is not great if it stores more than just hashForSymbol This only holds hash-for-symbol actually. This field does not include any flags. It is shifted with flagCount because we would like to adjust effective-bits-of-hash with StringImpl (which only offers 24bits hash). m_hashForSymbolShiftedWithFlagCount would be the precise name.

Created attachment 391454 [details] Patch

Created attachment 391456 [details] Patch

Comment on attachment 391456 [details] Patch I'll do a bit more.

Created attachment 391486 [details] Patch

Created attachment 391488 [details] Patch

Comment on attachment 391488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391488&action=review > Source/WTF/wtf/CompactPointerTuple.h:52 > + static_assert(WTF_OS_CONSTANT_EFFECTIVE_ADDRESS_WIDTH <= 48); This should be written as static_assert(OS_CONSTANT(EFFECTIVE_ADDRESS_WIDTH) <= 48); if possible.

Comment on attachment 391488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391488&action=review nice. r=me Mostly minor nits. Can you test this on arm64_32 to make sure nothing broke there? Might also be worth filing a bug for Igalia folks to look into 32-bit on their architectures > Source/JavaScriptCore/ChangeLog:8 > + This patch shrinks sizeof(Structure) from 112 to 96 (16 bytes) in 64 bit architectures. it's not 64-bit architectures, its architectures with 64-bit pointers. (Since arm64_32 does not do this) > Source/JavaScriptCore/ChangeLog:13 > + 1. StringHasher is always generating 24 bits hashes so far. By leveraging this fact, > + we store hash code in Structure in 3 bytes (24 bits) for property-hash and seen-property-hash. the code isn't actually doing this. AFAICT, it uses 48 bits for seen properties, and uses 16 bits to accumulate hash, and then uses the 64-bit combination of those things as the "seen property hash" (On a side note, I wonder if we really even need the seen property hash anymore, since the bloom filter might be sufficient. But I guess if we have the spare bits, it doesn't hurt to keep it.) > Source/JavaScriptCore/ChangeLog:20 > + We are setting m_cachedPrototypeChain only if Structure is for JSObject. Clearing happens only if it is not "are" => "were". "happens" => "happened" "it is not" => "it was not" > Source/JavaScriptCore/ChangeLog:25 > + Combining all of the above techniques and gets us 16 bytes. "and gets us" => "saves us" > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6094 > + LValue cachedPrototypeChainOrRareData = m_out.bitAnd(m_out.constIntPtr(Structure::cachedPrototypeChainOrRareDataMask), m_out.loadPtr(structure, m_heaps.Structure_cachedPrototypeChainOrRareData)); only do the and if address64? (and a few other places too in this file) You could even have a helper that masks out on address64 architectures > Source/JavaScriptCore/jit/AssemblyHelpers.h:1523 > +#if CPU(ADDRESS64) // Not USE(JSVALUE64). Structure::m_classInfo's implementation is changed by size of pointer. not sure this comment is needed given you do this everywhere without this comment > Source/JavaScriptCore/runtime/JSObjectInlines.h:219 > + [&] (const GCSafeConcurrentJSCellLocker&, PropertyOffset offset, PropertyOffset newMaxOffset) { nit: also fix style like you did elsewhere? > Source/JavaScriptCore/runtime/Structure.h:920 > + // Structure is one of the most frequently allocated data structure. Moreover, Structure tends to be alive long! "alive long" => "alive a long time" > Source/JavaScriptCore/runtime/Structure.h:929 > + // Other pairs works well. We carefully put assertions to setters, analyze access patterns and pick appropriate pairs in Structure fields. nice > Source/JavaScriptCore/runtime/StructureInlines.h:162 > +inline bool Structure::ruleOutUnseenPropertyHash(UniquedStringImpl* uid) const this should be called `ruleOutUnseenProperty`. It has nothing to do with hashes. > Source/JavaScriptCore/runtime/StructureInlines.h:177 > +#if CPU(ADDRESS64) > + return bitwise_cast<uintptr_t>(m_propertyHashAndSeenProperties.pointer()); > +#else > + return m_seenProperties; > +#endif minor style nit: I'd make this just return a TinyBloomFilter, and the code inside ruleOut can just call ruleOut. > Source/WTF/wtf/CompactPointerTuple.h:57 > + return 48 / 8; should we give 48 a name instead of using it directly everywhere? Maybe maxNumberOfBitsInPointer? > Source/WTF/wtf/CompactRefPtrTuple.h:34 > +class CompactRefPtrTuple final { nice > LayoutTests/ChangeLog:9 > + This test is half-broken since it relies on HashMap's order implicitly. > + We changed SymbolImpl's hash code, so it makes the result different. can you file a bug so we can make the test not reliant one this?

Comment on attachment 391488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391488&action=review Thanks! >> Source/JavaScriptCore/ChangeLog:8 >> + This patch shrinks sizeof(Structure) from 112 to 96 (16 bytes) in 64 bit architectures. > > it's not 64-bit architectures, its architectures with 64-bit pointers. (Since arm64_32 does not do this) Fixed. >> Source/JavaScriptCore/ChangeLog:13 >> + we store hash code in Structure in 3 bytes (24 bits) for property-hash and seen-property-hash. > > the code isn't actually doing this. AFAICT, it uses 48 bits for seen properties, and uses 16 bits to accumulate hash, and then uses the 64-bit combination of those things as the "seen property hash" > > (On a side note, I wonder if we really even need the seen property hash anymore, since the bloom filter might be sufficient. But I guess if we have the spare bits, it doesn't hurt to keep it.) Yes. I've just removed this (1). >> Source/JavaScriptCore/ChangeLog:20 >> + We are setting m_cachedPrototypeChain only if Structure is for JSObject. Clearing happens only if it is not > > "are" => "were". > "happens" => "happened" > "it is not" => "it was not" Fixed. >> Source/JavaScriptCore/ChangeLog:25 >> + Combining all of the above techniques and gets us 16 bytes. > > "and gets us" => "saves us" Fixed. >> Source/JavaScriptCore/jit/AssemblyHelpers.h:1523 >> +#if CPU(ADDRESS64) // Not USE(JSVALUE64). Structure::m_classInfo's implementation is changed by size of pointer. > > not sure this comment is needed given you do this everywhere without this comment Removed. >> Source/JavaScriptCore/runtime/JSObjectInlines.h:219 >> + [&] (const GCSafeConcurrentJSCellLocker&, PropertyOffset offset, PropertyOffset newMaxOffset) { > > nit: also fix style like you did elsewhere? Fixed. >> Source/JavaScriptCore/runtime/Structure.h:920 >> + // Structure is one of the most frequently allocated data structure. Moreover, Structure tends to be alive long! > > "alive long" => "alive a long time" Fixed. >> Source/JavaScriptCore/runtime/StructureInlines.h:162 >> +inline bool Structure::ruleOutUnseenPropertyHash(UniquedStringImpl* uid) const > > this should be called `ruleOutUnseenProperty`. It has nothing to do with hashes. Fixed. >> Source/JavaScriptCore/runtime/StructureInlines.h:177 >> +#endif > > minor style nit: I'd make this just return a TinyBloomFilter, and the code inside ruleOut can just call ruleOut. Fixed. >> Source/WTF/wtf/CompactPointerTuple.h:52 >> + static_assert(WTF_OS_CONSTANT_EFFECTIVE_ADDRESS_WIDTH <= 48); > > This should be written as static_assert(OS_CONSTANT(EFFECTIVE_ADDRESS_WIDTH) <= 48); if possible. Fixed. >> Source/WTF/wtf/CompactPointerTuple.h:57 >> + return 48 / 8; > > should we give 48 a name instead of using it directly everywhere? > > Maybe maxNumberOfBitsInPointer? Fixed. >> LayoutTests/ChangeLog:9 >> + We changed SymbolImpl's hash code, so it makes the result different. > > can you file a bug so we can make the test not reliant one this? Filed https://bugs.webkit.org/show_bug.cgi?id=208118

Comment on attachment 391488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391488&action=review >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6094 >> + LValue cachedPrototypeChainOrRareData = m_out.bitAnd(m_out.constIntPtr(Structure::cachedPrototypeChainOrRareDataMask), m_out.loadPtr(structure, m_heaps.Structure_cachedPrototypeChainOrRareData)); > > only do the and if address64? (and a few other places too in this file) > > You could even have a helper that masks out on address64 architectures I think the current FTL does not assume non-address-64 environment. FTL supports address64 environment and does not support other environments. Just adding this helper is not enough to make FTL work with ARM64_32. For example, so many places are using Int64 / pointerType() in the same meaning. For now, I don't think we should assume non-address-64 case in FTL to make code simple.

Committed r257201: <https://trac.webkit.org/changeset/257201>

Committed r257212: <https://trac.webkit.org/changeset/257212>

Comment on attachment 391488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391488&action=review > Source/JavaScriptCore/runtime/Structure.h:754 > + m_maxOffsetAndTransitionPropertyName.setPointer(transitionPropertyName); We are missing 32-bit case here. This is causing build failures on ports where `CPU(ADDRESS64)` is false.

(In reply to Yusuke Suzuki from comment #27) > Committed r257212: <https://trac.webkit.org/changeset/257212> This fix what I mentioned. Thank you.

Committed r257237: <https://trac.webkit.org/changeset/257237>

Committed r257257: <https://trac.webkit.org/changeset/257257>

Committed r259463: <https://trac.webkit.org/changeset/259463>