...
Created attachment 383442 [details] Patch
Created attachment 383443 [details] Patch
Created attachment 383445 [details] Patch
Created attachment 383446 [details] Patch
Created attachment 383447 [details] Patch
Comment on attachment 383447 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383447&action=review > Source/JavaScriptCore/heap/BlockDirectory.cpp:351 > + [&] (auto, const char* name) { Are you intentionally passing by value here?
Comment on attachment 383447 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383447&action=review >> Source/JavaScriptCore/heap/BlockDirectory.cpp:351 >> + [&] (auto, const char* name) { > > Are you intentionally passing by value here? Yes, this should be BlockDirectoryBitVectorRef. But I'm planning to change this to BlockDirectoryBitVectorRef to make it explicit.
Comment on attachment 383447 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383447&action=review >>> Source/JavaScriptCore/heap/BlockDirectory.cpp:351 >>> + [&] (auto, const char* name) { >> >> Are you intentionally passing by value here? > > Yes, this should be BlockDirectoryBitVectorRef. But I'm planning to change this to BlockDirectoryBitVectorRef to make it explicit. Ah, we cannot. Each bitvector has different type.
Created attachment 383492 [details] Patch WIP
Created attachment 383495 [details] Patch WIP
Created attachment 383500 [details] Patch WIP
Created attachment 383507 [details] Patch
Created attachment 383514 [details] Patch WIP
Created attachment 383518 [details] Patch
Created attachment 383519 [details] Patch
Created attachment 383520 [details] Patch
Created attachment 383521 [details] Patch
Created attachment 383528 [details] Patch
Created attachment 383532 [details] Patch
Comment on attachment 383532 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383532&action=review r=me > Source/JavaScriptCore/heap/BlockDirectoryBits.h:156 > + for (unsigned i = BlockDirectoryBitVectorView<kind>::arrayLength(); i--;) Is there a reason for the previous function using Base::arrayLength() while this one is using BlockDirectoryBitVectorView<kind>::arrayLength() ? > Source/JavaScriptCore/heap/BlockDirectoryBits.h:191 > + BlockDirectoryBitVectorRef<Kind::capitalBitName> lowerBitName() \ I don't think this function is useful as long as you have the one above. Either delete it, or make it return a const BlockDirectoryBitVectorRef<Kind::capitalBitName>
Comment on attachment 383532 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383532&action=review Thanks! >> Source/JavaScriptCore/heap/BlockDirectoryBits.h:156 >> + for (unsigned i = BlockDirectoryBitVectorView<kind>::arrayLength(); i--;) > > Is there a reason for the previous function using Base::arrayLength() while this one is using BlockDirectoryBitVectorView<kind>::arrayLength() ? No reason. Thanks, fixed. >> Source/JavaScriptCore/heap/BlockDirectoryBits.h:191 >> + BlockDirectoryBitVectorRef<Kind::capitalBitName> lowerBitName() \ > > I don't think this function is useful as long as you have the one above. > Either delete it, or make it return a const BlockDirectoryBitVectorRef<Kind::capitalBitName> Yeah, removing View version, since Ref one is used to modify the bits.
Comment on attachment 383532 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383532&action=review >>> Source/JavaScriptCore/heap/BlockDirectoryBits.h:191 >>> + BlockDirectoryBitVectorRef<Kind::capitalBitName> lowerBitName() \ >> >> I don't think this function is useful as long as you have the one above. >> Either delete it, or make it return a const BlockDirectoryBitVectorRef<Kind::capitalBitName> > > Yeah, removing View version, since Ref one is used to modify the bits. I tried removing this but it brings much complexity like implementing `bool operator[] const`. For now, I'll just keep it since it just works anyway :)
Committed r252452: <https://trac.webkit.org/changeset/252452>
<rdar://problem/57184145>