[Webkit-unassigned] [Bug 143999] Remove AllocationProfileWatchpoint node

[bugzilla-daemon at webkit.org]

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

--- Comment #5 from Filip Pizlo <fpizlo at apple.com> ---
(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 251246 [details]
> > Tentative patch, non thread-safe
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=251246&action=review
> > 
> > >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2643
> > >>                  if (Structure* structure = function->allocationStructure()) {
> > > 
> > > We probably need to freeze the rare data here, in case it is removed by the JS thread and then the GC runs at that point.
> > > Not sure how to do this, I first need to investigate more deeply how freeze() & the GC works.
> > 
> > It seems that you're using JSFunction's helpers to get things from the rare
> > data.  I would make this explicit here, so instead of
> > function->allocationStructure() you'd do something like
> > rareData->allocationStructure().  And instead of
> > function->allocationProfileWatchpointSet() you'd do
> > rareData->allocationProfileWatchpointSet().
> 
> I had this discussion with Michael yesterday who told me keeping the
> accessors would be better, but that is indeed problematic here. I will
> change that.
> 
> > 
> > Then you'd do:
> > 
> > FunctionRareData* rareData = function->rareData();
> > if (rareData) {
> >     if (Structure* structure = rareData->structure()) {
> >         m_graph.freeze(rareData);
> 
> Right, the m_graph.freeze(rareData) was the thing I was not sure would be
> sufficient (thought I needed to store it somewhere).
> Shouldn't it rather be:
> 
> if (rareData) {
>     m_graph.freeze(rareData);
>     if (Structure* structure = rareData->structure()) {
>       ...
>     }
> }
> 
> in case the rare data is deallocated in between the two ifs (this shouldn't
> matter after https://bugs.webkit.org/show_bug.cgi?id=144000 is taken care
> of)?

It's safe to assume that a GC cannot happen during a DFG phase.  The parser counts as a phase.  Hence, you only need to freeze it if you decide that you'll be using it after parsing completes - i.e. exactly if/when you addLazily().

> 
> Thanks!

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150421/55cb23ed/attachment-0001.html>