Bug 193612

Summary: [JSC] Shrink data structure size in JSC/heap
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 193606    
Attachments:
Description Flags
Patch saam: review+

Description Yusuke Suzuki 2019-01-19 03:18:35 PST
[JSC] Shrink data structure size in JSC/heap
Comment 1 Yusuke Suzuki 2019-01-19 04:01:50 PST
Created attachment 359603 [details]
Patch
Comment 2 Saam Barati 2019-01-20 12:05:59 PST
Comment on attachment 359603 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359603&action=review

> Source/JavaScriptCore/heap/Heap.h:581
> +    Markable<CollectionScope, EnumMarkableTraits<CollectionScope>> m_collectionScope;

I really hate the name of this "Markable" class. Why don't we just encode this stuff in Optional via some extra type parameter?
Comment 3 Yusuke Suzuki 2019-01-20 12:37:18 PST
Comment on attachment 359603 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359603&action=review

Thanks!

>> Source/JavaScriptCore/heap/Heap.h:581
>> +    Markable<CollectionScope, EnumMarkableTraits<CollectionScope>> m_collectionScope;
> 
> I really hate the name of this "Markable" class. Why don't we just encode this stuff in Optional via some extra type parameter?

This is because `WTF::Optional` was `std::optional` when `WTF::Markable` is introduced. We do not have the way to modify `std::optional` at that point.
Anyway, now we have a chance to do that. Opened the bug for that. https://bugs.webkit.org/show_bug.cgi?id=193625
Comment 4 Yusuke Suzuki 2019-01-20 12:39:38 PST
Committed r240216: <https://trac.webkit.org/changeset/240216>
Comment 5 Radar WebKit Bug Importer 2019-01-20 12:40:29 PST
<rdar://problem/47416829>
Comment 6 Saam Barati 2019-01-20 12:40:41 PST
Comment on attachment 359603 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359603&action=review

>>> Source/JavaScriptCore/heap/Heap.h:581
>>> +    Markable<CollectionScope, EnumMarkableTraits<CollectionScope>> m_collectionScope;
>> 
>> I really hate the name of this "Markable" class. Why don't we just encode this stuff in Optional via some extra type parameter?
> 
> This is because `WTF::Optional` was `std::optional` when `WTF::Markable` is introduced. We do not have the way to modify `std::optional` at that point.
> Anyway, now we have a chance to do that. Opened the bug for that. https://bugs.webkit.org/show_bug.cgi?id=193625

Thanks. I also meant to say that this is completely unrelated to this patch. This patch LGTM