[webkit-reviews] review granted: [Bug 204919] The compiler thread should not adjust Identifier refCounts. : [Attachment 385055] proposed patch.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Dec 6 17:04:44 PST 2019
Saam Barati <sbarati at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 204919: The compiler thread should not adjust Identifier refCounts.
https://bugs.webkit.org/show_bug.cgi?id=204919
Attachment 385055: proposed patch.
https://bugs.webkit.org/attachment.cgi?id=385055&action=review
--- Comment #4 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 385055
--> https://bugs.webkit.org/attachment.cgi?id=385055
proposed patch.
View in context: https://bugs.webkit.org/attachment.cgi?id=385055&action=review
r=me
> Source/JavaScriptCore/dfg/DFGPlan.cpp:712
> + // a cancelled plan needs to keep its m_vm pointer to let the mutator
know ]
remove "]"
> Source/JavaScriptCore/dfg/DFGPlan.cpp:717
> + // Similarly, plans may be cancelled from the GC thread. In that case,
the
> + // treatment is same as the above except that the cancelled plan will be
> + // kept alive in the worklist's m_keptAliveCancelledPlansFromGCThread.
when the vector's are unified, this paragraph can be omitted
> Source/JavaScriptCore/dfg/DFGPlan.cpp:723
> + // We rely on this all over the place to filter out Cancelled plans.
you should say what "this" is. Specifically, a bit comparison against VM
pointer fails
> Source/JavaScriptCore/dfg/DFGWorklist.cpp:99
> + bool isInMutator =
m_plan->unnukedVM()->currentThreadIsHoldingAPILock();
let's remove this and just always add it to the vector based on what we talked
about
> Source/JavaScriptCore/dfg/DFGWorklist.cpp:311
> +void removeCancelledPlans(VM& vm, HashSet<RefPtr<Plan>>& removedPlans,
Vector& cancelledPlans)
once we have one vector, let's just put this below
> Source/JavaScriptCore/dfg/DFGWorklist.cpp:330
> + HashSet<RefPtr<Plan>> removedPlans;
once the function is moved in here, you can conditionalize this on
!ASSERT_DISABLED
> Source/JavaScriptCore/dfg/DFGWorklist.h:118
> + Vector<RefPtr<Plan>, 4> m_keptAliveCancelledPlansFromGCThread;
> + Vector<RefPtr<Plan>, 4> m_keptAliveCancelledPlansFromCompilerThread;
let's join these together and perhaps let's call it something like
"m_cancelledPlansPendingDestruction" or something like that
More information about the webkit-reviews
mailing list