Bug 182226

Summary:

LargeAllocation should do the same distancing as MarkedBlock

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ggaren, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, saam, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

181636

Attachments:

Description	Flags
the patch	saam: review+

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.