Bug 120615

Created attachment 210415 [details] the patch

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.

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

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