[Webkit-unassigned] [Bug 71977] DFG should not reparse code that was just parsed

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 10 12:06:46 PST 2011


https://bugs.webkit.org/show_bug.cgi?id=71977


Geoffrey Garen <ggaren at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #114468|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #8 from Geoffrey Garen <ggaren at apple.com>  2011-11-10 12:06:47 PST ---
(From update of attachment 114468)
View in context: https://bugs.webkit.org/attachment.cgi?id=114468&action=review

r=me, with some comments below.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1655
> +    if (hasInstructions() && m_shouldDiscardBytecodeLater)

The "Later" in "m_shouldDiscardBytecodeLater" confused me because it sounded like setting the flag was a way to delay or prevent discarding bytecode ("Not now, later"), but actually it's the way to cause bytecode to be discarded. I think "m_shouldDiscardBytecode" would be better.

> Source/JavaScriptCore/dfg/DFGByteCodeCache.h:68
> +        // This is likely to be good enough.

Sneaky, but true. I would remove this comment, or add a "why" explanation.

Thinking this through, I wonder if | might be clearer than ^ here. This hash function really just relies on the low bits of m_executable being zero, and the low bits of m_kind being meaningful.

> Source/JavaScriptCore/heap/ListableHandler.h:60
> +        void addNotThreadSafe(T* handler)

Please make this private.

> Source/JavaScriptCore/heap/UltraWeakFinalizer.h:37
> +class UltraWeakFinalizer : public ListableHandler<UltraWeakFinalizer> {

The "Ultra" in "UltraWeak" didn't do enough to tell me what this class is. To my mind, two things are unique about this kind of "finalizer": (1) It fires no matter what, even if the owning object is live; (2) If GC happens soon enough, it might prefer not to fire, thus avoiding cache churn. Based on those facts, I'd suggest:

- CacheOwner (with a "clearCache" callback)
OR
- UnconditionalFinalizer (with a "finalize" callback)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list