[Webkit-unassigned] [Bug 72312] DFG code blocks that have speculation checks on objects should refer to those objects weakly

Thu Nov 17 13:53:24 PST 2011

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

--- Comment #7 from Geoffrey Garen <ggaren at apple.com>  2011-11-17 13:53:24 PST ---
(From update of attachment 115538)
View in context: https://bugs.webkit.org/attachment.cgi?id=115538&action=review

Thinking abstractly, I'm not sure this is the right strategy for making optimizations weakly referenced. But perhaps there's a concrete application I'm not considering, which validates this strategy. I would have a much easier time reasoning about this code if I could look at a benchmark or test case that demonstrated the problem we're trying to solve.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1623
> +    // Add a weak reference harvester if we have not reached fixpoint and need to
> +    // run again.

To turn this into a "why" comment, I would write: GC doesn't have enough information yet for us to decide whether to keep our DFG data, so we need to register a handler to run again at the end of GC, when more information is available.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1657
> +                // If the following three things are live, then the target of the
> +                // transition is also live:
> +                // - This code block. We know it's live already because otherwise
> +                //   we wouldn't be scanning ourselves.
> +                // - The code origin of the transition. Transitions may arise from
> +                //   code that was inlined. They are not relevant if the user's
> +                //   object that is required for the inlinee to run is no longer
> +                //   live.
> +                // - The source of the transition. The transition checks if some
> +                //   heap location holds the source, and if so, stores the target.
> +                //   Hence the source must be live for the transition to be live.

Why do we want structure transitions to perform checks on behalf of inlined functions? It would be much more natural for functions themselves to act as first-class weak citizens, such that a linked/inlined function disappearing would be sufficient cause to blow away an optimized code block.

More broadly, I don't understand why structure transitions get special treatment here. Why are they distinct from other weakly referenced optimizations in a DFG code block?

I'm concerned that an A->B transition keeps B alive if A is alive. This is the opposite of the structure marking strategy, which says that A should stay alive only if B is alive. The benefit of the structure marking strategy is that it ensures that a chain of structures can be retired if it becomes stale. In the case of object literals, A is the default empty object structure, which is permanent, so, with this patch's strategy, all transitions from an object literal are permanent. In the case of a constructor function, A is the function's .prototype's inheritorID, which will have been marked by the same object marking the CodeBlock, so, with this patch's strategy, the transition once again becomes permanent. (To clarify, by "permanent" I mean, "will live as long as a strong reference", not necessarily "memory leak".)

Maybe you're trying to make sure that a hot constructor CodeBlock survives even if the objects it constructs tend to be garbage? If so, I'm still not sure why the transitions are special compared to other weakly optimized assumptions. It seems like any weakly optimized assumption could pertain to a typically short-lived object, and cause jettison churn. Perhaps a better strategy would be an explicit jettison churn guard, instead.

> Source/JavaScriptCore/bytecode/CodeBlock.h:1111
> +            // Am I a DFG code block? If not, then I'm live if I am being scanned.
> +            if (!m_dfgData)
> +                return true;
> +            
> +            // If I am a DFG code block, then am I currently executing? If so,
> +            // then I'm definitely live.
> +            if (m_dfgData->mayBeExecuting)

I don't think these "what" comments add anything. A "why" comment explaining why baseline JIT code always assumes itself to be live might be helpful.

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