Many call sites use makeNeverDestroyed, but could instead take advantage of the NeverDestroyed deduction guide
Created attachment 447271 [details] Patch
Created attachment 447277 [details] Patch
Created attachment 447287 [details] Patch
Created attachment 447298 [details] Patch
Comment on attachment 447287 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447287&action=review r=me, but it should probably build on all the bots first. > Source/WebCore/Modules/mediasession/MediaSession.cpp:80 > + static constexpr SortedArrayMap map { mappings }; Nice. > Source/WebCore/page/DebugPageOverlays.cpp:155 > + static NeverDestroyed regionColors { MemoryCompactLookupOnlyRobinHoodHashMap<String, SRGBA<uint8_t>> { { This should really be converted to a SortedArrayMap at some point. > Source/WebCore/page/PerformanceUserTiming.cpp:46 > + static NeverDestroyed map = MemoryCompactLookupOnlyRobinHoodHashMap<String, NavigationTimingFunction> { Can this use a SortedArrayMap as well? (not sure what would happen with the member function pointers). > Source/WebCore/platform/graphics/FontCascade.cpp:376 > + static NeverDestroyed map = MemoryCompactLookupOnlyRobinHoodHashSet<AtomString> { I wonder if the AtomicString hashing optimization is actually faster than a SortedArraySet here. (it would be good to have a rough feeling of where the cutoff is). > Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:1767 > + static NeverDestroyed propertyKeyName = [] { If this is only used on apple platforms (e.g. not on Windows), it might make sense to convert this objective-c and use @[] which is even more constant. > Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp:559 > HashSet<String, ASCIICaseInsensitiveHash> types; I think this could be simplified to: static NeverDestroyed types = HashSet<String, ASCIICaseInsensitiveHash> { #if ENABLE(VP9) "video/webm"; #endif #if ENABLE(VORBIS) || ENABLE(OPUS) "audio/webm"; #endif }; or something thereabouts. That said, the only user of this just iterates the whole set, so it could just be a Span of a constexpr array.
Comment on attachment 447287 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447287&action=review >> Source/WebCore/page/DebugPageOverlays.cpp:155 >> + static NeverDestroyed regionColors { MemoryCompactLookupOnlyRobinHoodHashMap<String, SRGBA<uint8_t>> { { > > This should really be converted to a SortedArrayMap at some point. I thought to myself: Need to look at every use of MemoryCompactLookupOnlyRobinHoodHashMap, at least, to see if SortedArrayMap is better suited. I worry that the hashing may still be a little faster, so it’s not always an automatic win, but the sorted array will definitely use less memory. Probably will change many of them. >> Source/WebCore/page/PerformanceUserTiming.cpp:46 >> + static NeverDestroyed map = MemoryCompactLookupOnlyRobinHoodHashMap<String, NavigationTimingFunction> { > > Can this use a SortedArrayMap as well? (not sure what would happen with the member function pointers). It absolutely can. Should work fine with those pointers. The reason I didn’t do the conversion was my uncertainty mentioned above. >> Source/WebCore/platform/graphics/FontCascade.cpp:376 >> + static NeverDestroyed map = MemoryCompactLookupOnlyRobinHoodHashSet<AtomString> { > > I wonder if the AtomicString hashing optimization is actually faster than a SortedArraySet here. (it would be good to have a rough feeling of where the cutoff is). Great insight. I would indeed love to use SortedArraySet instead if it’s a win. >> Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:1767 >> + static NeverDestroyed propertyKeyName = [] { > > If this is only used on apple platforms (e.g. not on Windows), it might make sense to convert this objective-c and use @[] which is even more constant. I think it’s used only on Windows. >> Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp:559 >> HashSet<String, ASCIICaseInsensitiveHash> types; > > I think this could be simplified to: > > static NeverDestroyed types = HashSet<String, ASCIICaseInsensitiveHash> { > #if ENABLE(VP9) > "video/webm"; > #endif > #if ENABLE(VORBIS) || ENABLE(OPUS) > "audio/webm"; > #endif > }; > > > or something thereabouts. > > That said, the only user of this just iterates the whole set, so it could just be a Span of a constexpr array. Both great points. I will come back and improve this after landing the patch.
Committed r287138 (?): <https://commits.webkit.org/r287138>
<rdar://problem/86578814>