Bug 182226 - LargeAllocation should do the same distancing as MarkedBlock
Summary: LargeAllocation should do the same distancing as MarkedBlock
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords: InRadar
Depends on:
Blocks: 181636
  Show dependency treegraph
 
Reported: 2018-01-28 15:52 PST by Filip Pizlo
Modified: 2018-01-28 21:49 PST (History)
9 users (show)

See Also:


Attachments
the patch (6.55 KB, patch)
2018-01-28 16:10 PST, Filip Pizlo
saam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2018-01-28 15:52:58 PST
Patch forthcoming.
Comment 1 Radar WebKit Bug Importer 2018-01-28 15:53:19 PST
<rdar://problem/36968095>
Comment 2 Filip Pizlo 2018-01-28 16:10:57 PST
Created attachment 332498 [details]
the patch
Comment 3 Saam Barati 2018-01-28 20:58:10 PST
Comment on attachment 332498 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=332498&action=review

> Source/JavaScriptCore/heap/LargeAllocation.cpp:41
> +    size_t sizeIncludingDistancing = sizeBeforeDistancing + distancing;

We don’t have any JIT code that needs to be updated to also do this? Do we not inline large allocations in the JIT?
Comment 4 Filip Pizlo 2018-01-28 21:07:05 PST
(In reply to Saam Barati from comment #3)
> Comment on attachment 332498 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=332498&action=review
> 
> > Source/JavaScriptCore/heap/LargeAllocation.cpp:41
> > +    size_t sizeIncludingDistancing = sizeBeforeDistancing + distancing;
> 
> We don’t have any JIT code that needs to be updated to also do this? Do we
> not inline large allocations in the JIT?

Only small allocations are inlined.  Large allocations are very complicated to do.

Here's a GC thinking trick: the cost of an allocation "hides behind" the cost of initializing every element. Because any not-totally-dumb allocation would be followed by at least an initialization of every element.

Large allocations mean allocating at least 8000 bytes.

Whether or not you inline a function call has infinitesimal cost compared to the cost of initializing 8000 bytes.

Therefore, we don't inline large allocations.
Comment 5 Filip Pizlo 2018-01-28 21:08:47 PST
Landed in Landed in https://trac.webkit.org/changeset/227721/webkit
Comment 6 Saam Barati 2018-01-28 21:49:35 PST
(In reply to Filip Pizlo from comment #4)
> (In reply to Saam Barati from comment #3)
> > Comment on attachment 332498 [details]
> > the patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=332498&action=review
> > 
> > > Source/JavaScriptCore/heap/LargeAllocation.cpp:41
> > > +    size_t sizeIncludingDistancing = sizeBeforeDistancing + distancing;
> > 
> > We don’t have any JIT code that needs to be updated to also do this? Do we
> > not inline large allocations in the JIT?
> 
> Only small allocations are inlined.  Large allocations are very complicated
> to do.
> 
> Here's a GC thinking trick: the cost of an allocation "hides behind" the
> cost of initializing every element. Because any not-totally-dumb allocation
> would be followed by at least an initialization of every element.
> 
> Large allocations mean allocating at least 8000 bytes.
> 
> Whether or not you inline a function call has infinitesimal cost compared to
> the cost of initializing 8000 bytes.
> 
> Therefore, we don't inline large allocations.

Makes sense.