Bug 194939 – [JSC] SmallStringsStorage is unnecessary

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

[Yusuke Suzuki]

[JSC] SmallStringsStorage is unnecessary

[Yusuke Suzuki]

Created attachment 362709 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 362709 [details]
Patch

Oops, fixing build issue.

[Yusuke Suzuki]

Created attachment 362710 [details]
Patch

[Yusuke Suzuki]

Created attachment 362711 [details]
Patch

[Mark Lam]

Comment on attachment 362711 [details]
Patch

r=me

[Geoffrey Garen]

Comment on attachment 362711 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362711&action=review

> Source/JavaScriptCore/runtime/SmallStrings.h:76
> +    void setCanAccessHeap(bool canAccessHeap) { m_canAccessHeap = canAccessHeap; }

I would call this isInitialized / setIsInitialized.

> Source/JavaScriptCore/runtime/VM.cpp:543
> +    smallStrings.setCanAccessHeap(false);

Is there code that tries to access the SmallStrings cache inside lastChanceToFinalize? If so, that feels like a bug.

[Yusuke Suzuki]

Comment on attachment 362711 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362711&action=review

>> Source/JavaScriptCore/runtime/SmallStrings.h:76
>> +    void setCanAccessHeap(bool canAccessHeap) { m_canAccessHeap = canAccessHeap; }
> 
> I would call this isInitialized / setIsInitialized.

OK, changed.

>> Source/JavaScriptCore/runtime/VM.cpp:543
>> +    smallStrings.setCanAccessHeap(false);
> 
> Is there code that tries to access the SmallStrings cache inside lastChanceToFinalize? If so, that feels like a bug.

I don't see the actual code is now using it. But I think touching smallStrings after VM Heap is destroyed can be possible.
This is because Identifier creation touches vm.smallStrings to get single character AtomicStringImpl. I think it is a possible story that we create Identifier after VM Heap is destroyed, because Identifier seems unrelated to VM GC Heap.
So I take safer design here.

[Yusuke Suzuki]

Committed r241954: <https://trac.webkit.org/changeset/241954>

[Radar WebKit Bug Importer]

<rdar://problem/48317238>

[Yusuke Suzuki]

Committed r241955: <https://trac.webkit.org/changeset/241955>