Bug 120615 – CodeBlock memory cost reporting should be rationalized

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

[Filip Pizlo]

CodeBlock should report the size of the bytecode as part of extra memory cost.  Also, CodeBlock should report the extra memory usage during GC as well so we don't forget that it's using memory.

[Filip Pizlo]

Created attachment 210415 [details]
the patch

[Darin Adler]

Comment on attachment 210415 [details]
the patch

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

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1851
> +    m_heap->reportExtraMemoryCost(
> +        sizeof(CodeBlock) + m_instructions.size() * sizeof(Instruction));

Why the strange line breaking? Looks like this all would fit on one line just fine.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1940
> +        visitor.reportExtraMemoryUsage(
> +            m_instructions.size() * sizeof(Instruction) / m_instructions.refCount());

Same line breaking comment. Also, the division by reference count is sufficiently non-obvious that I would include a comment, unless this is a common JavaScriptCore idiom that I am unaware of.

[Filip Pizlo]

(In reply to comment #2)
> (From update of attachment 210415 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210415&action=review
> 
> > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1851
> > +    m_heap->reportExtraMemoryCost(
> > +        sizeof(CodeBlock) + m_instructions.size() * sizeof(Instruction));
> 
> Why the strange line breaking? Looks like this all would fit on one line just fine.

Heh, you're probably right.  I don't like super long lines.  But I decide this rather arbitrarily - it depends on how wide my emacs window is at the time.  I'll un-wrap these.

> 
> > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1940
> > +        visitor.reportExtraMemoryUsage(
> > +            m_instructions.size() * sizeof(Instruction) / m_instructions.refCount());
> 
> Same line breaking comment. Also, the division by reference count is sufficiently non-obvious that I would include a comment, unless this is a common JavaScriptCore idiom that I am unaware of.

It's pseudo-common.  The idea is that the m_instructions is a shared array that is owned by multiple CodeBlocks.  Each CodeBlock gets traced by GC, and each one will make a reportExtraMemoryUsage() call.  But we only want to count the size of the instructions once.  The simplest way of doing that is to have each of them do the reporting but divide by the refCount.

We do this in one other place - accounting for memory used by JSStrings.  There we do something like this - take the StringImpl's size and divide by refCount.  Plus some other cruft to take into account immortal strings...

A comment could probably help.  It's not super obvious that m_instructions is a shared thing.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/155021