Bug 204039

Summary: [JSC] Put more things in IsoSubspace
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, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 204124    
Bug Blocks:    
Attachments:
Description Flags
Patch keith_miller: review+

Yusuke Suzuki
Reported 2019-11-08 18:57:18 PST
[JSC] Put more things in IsoSubspace
Attachments
Patch (15.00 KB, patch)
2019-11-08 18:58 PST, Yusuke Suzuki 		keith_miller: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2019-11-08 18:58:47 PST
Created attachment 383193 [details] Patch
Radar WebKit Bug Importer
Comment 2 2019-11-08 19:00:59 PST
<rdar://problem/57042686>
Keith Miller
Comment 3 2019-11-12 18:13:58 PST
Comment on attachment 383193 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383193&action=review r=me. > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:116 > + template<typename, SubspaceAccess> > + static IsoSubspace* subspaceFor(VM&) { return nullptr; } What does this do?
Saam Barati
Comment 4 2019-11-12 18:14:14 PST
Comment on attachment 383193 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383193&action=review > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:116 > + static IsoSubspace* subspaceFor(VM&) { return nullptr; } ASSERT_NOT_REACHED?
Yusuke Suzuki
Comment 5 2019-11-12 18:18:13 PST
Comment on attachment 383193 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383193&action=review Thanks! >>> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:116 >>> + static IsoSubspace* subspaceFor(VM&) { return nullptr; } >> >> What does this do? > > ASSERT_NOT_REACHED? This enforces us to define subspaceFor in derived classes of this UnlinkedCodeBlock. ASSERT_NOT_REACHED sounds nice, fixed!
Keith Miller
Comment 6 2019-11-12 18:22:50 PST
(In reply to Yusuke Suzuki from comment #5) > Comment on attachment 383193 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383193&action=review > > Thanks! > > >>> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:116 > >>> + static IsoSubspace* subspaceFor(VM&) { return nullptr; } > >> > >> What does this do? > > > > ASSERT_NOT_REACHED? > > This enforces us to define subspaceFor in derived classes of this > UnlinkedCodeBlock. ASSERT_NOT_REACHED sounds nice, fixed! Ah, I see. Too bad `= 0` doesn't work for non-virtual methods... ASSERT_NOT_REACHED sounds good.
Yusuke Suzuki
Comment 7 2019-11-12 18:25:51 PST
Committed r252390: <https://trac.webkit.org/changeset/252390>
Mark Lam
Comment 8 2019-11-13 09:48:49 PST
Comment on attachment 383193 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383193&action=review > Source/JavaScriptCore/runtime/VM.h:387 > + IsoSubspace stringSpace; > + IsoSubspace ropeStringSpace; Can you sort this alphabetically?
Yusuke Suzuki
Comment 9 2019-11-13 11:36:00 PST
Comment on attachment 383193 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383193&action=review >> Source/JavaScriptCore/runtime/VM.h:387 >> + IsoSubspace ropeStringSpace; > > Can you sort this alphabetically? Sure, I'll do it in https://bugs.webkit.org/show_bug.cgi?id=204144.
Note You need to log in before you can comment on or make changes to this bug.