Bug 207087 – [JSC] Introduce UnlinkedCodeBlockGenerator and reduce sizeof(UnlinkedCodeBlock)

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yusuke Suzuki]

UnlinkedCodeBlock has many Vectors while they are already frozen.
We should introduce UnlinkedCodeBlockWriter, and use Vector in it.
And when creating UnlinkedCodeBlock, we should use RefCountedArray for Vectors.

[Yusuke Suzuki]

Created attachment 389635 [details]
Patch

[Yusuke Suzuki]

I think this can offer sub-1% memory reduction in Gmail.

[Yusuke Suzuki]

Ah,dead-lock! Fixing

[Yusuke Suzuki]

Created attachment 389639 [details]
Patch

[Yusuke Suzuki]

Created attachment 389640 [details]
Patch

[Yusuke Suzuki]

Created attachment 389645 [details]
Patch

[Tadeu Zagallo]

Comment on attachment 389645 [details]
Patch

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

Nice!

> Source/JavaScriptCore/bytecode/UnlinkedCodeBlockGenerator.cpp:150
> +        if (!m_codeBlock->m_rareData) {

Why would the code block have rare data at this point?

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:291
> +    m_codeBlock->finalize(m_writer.finalize());

Not necessarily on this patch, but ideally I think UnlinkedCodeBlockGenerator should own the UnlinkedCodeBlock and return it from finalize.

> Source/WTF/wtf/RefCountedArray.h:201
> +    T& front() { return (*this)[0]; }
> +    const T& front() const { return (*this)[0]; }

I believe this called `first` in Vector, should we just call the same here?

[Yusuke Suzuki]

Comment on attachment 389645 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/bytecode/UnlinkedCodeBlockGenerator.cpp:150
>> +        if (!m_codeBlock->m_rareData) {
> 
> Why would the code block have rare data at this point?

If `NeedsClassFieldInitializer::Yes` is specified, we create a rareData in UnlinkedCodeBlock's constructor.

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:291
>> +    m_codeBlock->finalize(m_writer.finalize());
> 
> Not necessarily on this patch, but ideally I think UnlinkedCodeBlockGenerator should own the UnlinkedCodeBlock and return it from finalize.

Right! Filed a issue here. https://bugs.webkit.org/show_bug.cgi?id=207212

>> Source/WTF/wtf/RefCountedArray.h:201
>> +    const T& front() const { return (*this)[0]; }
> 
> I believe this called `first` in Vector, should we just call the same here?

Nice, fixed.

[Yusuke Suzuki]

Committed r255687: <https://trac.webkit.org/changeset/255687>

[Radar WebKit Bug Importer]

<rdar://problem/59155888>