Bug 193612 - [JSC] Shrink data structure size in JSC/heap
Summary: [JSC] Shrink data structure size in JSC/heap
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks: 193606
  Show dependency treegraph
 
Reported: 2019-01-19 03:18 PST by Yusuke Suzuki
Modified: 2019-01-20 18:57 PST (History)
6 users (show)

See Also:


Attachments
Patch (29.44 KB, patch)
2019-01-19 04:01 PST, Yusuke Suzuki
sbarati: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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