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+

Yusuke Suzuki
Reported 2019-01-19 03:18:35 PST
[JSC] Shrink data structure size in JSC/heap
Attachments
Patch (29.44 KB, patch)
2019-01-19 04:01 PST, Yusuke Suzuki 		saam: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2019-01-19 04:01:50 PST
Created attachment 359603 [details] Patch
Saam Barati
Comment 2 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?
Yusuke Suzuki
Comment 3 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
Yusuke Suzuki
Comment 4 2019-01-20 12:39:38 PST
Committed r240216: <https://trac.webkit.org/changeset/240216>
Radar WebKit Bug Importer
Comment 5 2019-01-20 12:40:29 PST
<rdar://problem/47416829>
Saam Barati
Comment 6 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
Note You need to log in before you can comment on or make changes to this bug.