Bug 212878

Created attachment 401289 [details]
Patch

Created attachment 401290 [details]
Patch

Hey Yusuke, just so I understand a little better, what is the benefit of removing this from the __DATA section?

(In reply to Sam Weinig from comment #3)
> Hey Yusuke, just so I understand a little better, what is the benefit of
> removing this from the __DATA section?

I believe the goal is to reduce dirty memory usage

(In reply to Saam Barati from comment #4)
> (In reply to Sam Weinig from comment #3)
> > Hey Yusuke, just so I understand a little better, what is the benefit of
> > removing this from the __DATA section?
> 
> I believe the goal is to reduce dirty memory usage

I guess to elaborate on how I model my understanding:
This will take away space from other things in DATA that could’ve theoretically fit on one page and perhaps bumping it into two.

(In reply to Saam Barati from comment #5)
> (In reply to Saam Barati from comment #4)
> > (In reply to Sam Weinig from comment #3)
> > > Hey Yusuke, just so I understand a little better, what is the benefit of
> > > removing this from the __DATA section?
> > 
> > I believe the goal is to reduce dirty memory usage
> 
> I guess to elaborate on how I model my understanding:
> This will take away space from other things in DATA that could’ve
> theoretically fit on one page and perhaps bumping it into two.

Yes, since __DATA is per page, it is possible that we construct a page like,


[<---------------------------historicUsageData-------------><-small data->]

In that case, if small data gets dirty, it taints entire page dirty even if historicUsageData part is not used at all.

(In reply to Yusuke Suzuki from comment #6)
> (In reply to Saam Barati from comment #5)
> > (In reply to Saam Barati from comment #4)
> > > (In reply to Sam Weinig from comment #3)
> > > > Hey Yusuke, just so I understand a little better, what is the benefit of
> > > > removing this from the __DATA section?
> > > 
> > > I believe the goal is to reduce dirty memory usage
> > 
> > I guess to elaborate on how I model my understanding:
> > This will take away space from other things in DATA that could’ve
> > theoretically fit on one page and perhaps bumping it into two.
> 
> Yes, since __DATA is per page, it is possible that we construct a page like,
> 
> 
> [<---------------------------historicUsageData-------------><-small data->]
> 
> In that case, if small data gets dirty, it taints entire page dirty even if
> historicUsageData part is not used at all.

Interesting. Would it ever make sense to annotate these rarely used large data portions with a specific section name to allow the linker to group rarely used data together?

(In reply to Sam Weinig from comment #7)
> (In reply to Yusuke Suzuki from comment #6)
> > (In reply to Saam Barati from comment #5)
> > > (In reply to Saam Barati from comment #4)
> > > > (In reply to Sam Weinig from comment #3)
> > > > > Hey Yusuke, just so I understand a little better, what is the benefit of
> > > > > removing this from the __DATA section?
> > > > 
> > > > I believe the goal is to reduce dirty memory usage
> > > 
> > > I guess to elaborate on how I model my understanding:
> > > This will take away space from other things in DATA that could’ve
> > > theoretically fit on one page and perhaps bumping it into two.
> > 
> > Yes, since __DATA is per page, it is possible that we construct a page like,
> > 
> > 
> > [<---------------------------historicUsageData-------------><-small data->]
> > 
> > In that case, if small data gets dirty, it taints entire page dirty even if
> > historicUsageData part is not used at all.
> 
> Interesting. Would it ever make sense to annotate these rarely used large
> data portions with a specific section name to allow the linker to group
> rarely used data together?

It is nice! So, if it is the data we need definitely, we should do that. But at the same time, by doing heap-allocation here, this patch reduces binary size :)

Committed r287238 (245398@trunk): <https://commits.webkit.org/245398@trunk>

<rdar://problem/86680604>

Funny, I did this same thing but found that it didn't change binary size.

The __DATA/_bss section is constructed at runtime (fresh pages mapped into memory). But if it has the side effect of shuffling around other things in __DATA then maybe it can affect binary size?

See bug 233476.

(In reply to Simon Fraser (smfr) from comment #11)
> Funny, I did this same thing but found that it didn't change binary size.
> 
> The __DATA/_bss section is constructed at runtime (fresh pages mapped into
> memory). But if it has the side effect of shuffling around other things in
> __DATA then maybe it can affect binary size?

Ah! Yes. But anyway, the original intent of this patch is avoid having dirty __DATA memory by purging large rarely used data from __DATA (as described in https://bugs.webkit.org/show_bug.cgi?id=212878#c6), and it is still held :)