Bug 204039 – [JSC] Put more things in IsoSubspace

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Put more things in IsoSubspace

[Yusuke Suzuki]

Created attachment 383193 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/57042686>

[Keith Miller]

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 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 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]

(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]

Committed r252390: <https://trac.webkit.org/changeset/252390>

[Mark Lam]

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 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.