Summary: | [JSC] Shrink data structure size in JSC/heap | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||
Component: | New Bugs | Assignee: | 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
Yusuke Suzuki
2019-01-19 03:18:35 PST
Created attachment 359603 [details]
Patch
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 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 Committed r240216: <https://trac.webkit.org/changeset/240216> 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 |