[webkit-reviews] review denied: [Bug 68329] DFG should support continuous optimization : [Attachment 108117] the patch - fix style

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 21 10:49:00 PDT 2011


Geoffrey Garen <ggaren at apple.com> has denied Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 68329: DFG should support continuous optimization
https://bugs.webkit.org/show_bug.cgi?id=68329

Attachment 108117: the patch - fix style
https://bugs.webkit.org/attachment.cgi?id=108117&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=108117&action=review


Looks like the Windows build is still angry:

2>c:\cygwin\home\buildbot\WebKit\Source\JavaScriptCore\wtf/BitVector.h(31) :
fatal error C1083: Cannot open include file: 'strings.h': No such file or
directory

> CodeBlocks can refer to a lot of stuff.  So we only want to scan them if
they're live.

Fair point.

> It's most efficient to have that scan include CodeBlock detection, so we're
only touching each location in the RegisterFile once during a collection.

Another fair point.

I'm happy with the solution you came up with. (Thinking about this, it occurred
to me that, long-term, if we GC-allocate CodeBlocks, the normal conservative
scan will "just work".)

r- for the Windows build.

> Source/JavaScriptCore/heap/Heap.cpp:504
> +    m_jettisonedCodeBlocks.traceCodeBlocks(visitor);

This should be followed by visitor.drain().

> Source/JavaScriptCore/heap/Heap.h:103
> +	   void addJettisonCodeBlock(PassOwnPtr<CodeBlock>);

Typo: Should be "addJettisonedCodeBlock", not "addJettisonCodeBlock".

> Source/JavaScriptCore/wtf/BitVector.h:96
> +	   if (m_bitsOrPointer >> maxInlineBits())

Should be "isInline()".


More information about the webkit-reviews mailing list