[JSC] Add VectorTraits for WriteBarrier #2492
Conversation
Constellation
added
JavaScriptCore
webkit-early-warning-system
added
the
merging-blocked
|
|
||
| template<typename T> struct VectorTraits<JSC::WriteBarrier<T>> : public SimpleClassVectorTraits { | ||
| static_assert(std::is_trivially_destructible<JSC::WriteBarrier<T>>::value); | ||
| static const bool canInitializeWithMemset = true; |
There was a problem hiding this comment.
Since canInitializeWithMemset is already true in the base class, I think we should omit this line.
| template<> struct VectorTraits<JSC::WriteBarrier<JSC::Unknown>> : public SimpleClassVectorTraits { | ||
| static_assert(std::is_trivially_destructible<JSC::WriteBarrier<JSC::Unknown>>::value); | ||
| #if USE(JSVALUE64) | ||
| // We can memset only in JSVALUE64 since empty value is zero. On the other hand, JSVALUE32_64's empty value is not zero. |
There was a problem hiding this comment.
Comment is great but code should be #if !USE(JSVALUE64) and only include setting to false, rely on inheritance for the true case.
| static const bool canInitializeWithMemset = false; | ||
| #endif | ||
| static const bool canCopyWithMemcpy = true; | ||
| }; |
There was a problem hiding this comment.
Can we also set canCompareWithMemcmp to true? Also seems like operator== for two WriteBarrier<Unknown> should not decode the two values, just compare them still encoded.
There was a problem hiding this comment.
Ah, canCompareWithMemcmp was true since SimpleClassVectorTraits is setting it to true :)
Constellation
removed
the
merging-blocked
Constellation
force-pushed
the
eng/JSC-Add-VectorTraits-for-WriteBarrier
branch
from
b96371f
to
61499f2
Compare
Constellation
added
the
unsafe-merge-queue
https://bugs.webkit.org/show_bug.cgi?id=196242 Reviewed by Darin Adler. WriteBarrier<> is one of the most frequently used class for Vector etc. in JSC. Let's add VectorTraits for that. * Source/JavaScriptCore/runtime/WriteBarrier.h: Canonical link: https://commits.webkit.org/252556@main
webkit-early-warning-system
force-pushed
the
eng/JSC-Add-VectorTraits-for-WriteBarrier
branch
from
61499f2
to
1eb3c9f
Compare
|
Committed 252556@main (1eb3c9f): https://commits.webkit.org/252556@main Reviewed commits have been landed. Closing PR #2492 and removing active labels. |
webkit-commit-queue
removed
the
unsafe-merge-queue
|
|
||
| template<typename T> struct VectorTraits<JSC::WriteBarrier<T>> : public SimpleClassVectorTraits { | ||
| static_assert(std::is_trivially_destructible<JSC::WriteBarrier<T>>::value); | ||
| static constexpr bool canCopyWithMemcpy = true; |
There was a problem hiding this comment.
@Constellation Isn't this wrong? My understanding is that memcpy is free to use load/store instructions with fewer than 8 bytes, which would cause GC tearing. I know we do something special for some copies, what is the current state of that?
1eb3c9f