PartitionAlloc: API changes to support new partitionPurgeMemoryGeneric() mode.

PartitionAlloc: API changes to support new partitionPurgeMemoryGeneric() mode.

No real functional changes.

BUG=501878
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197406

[Chris Evans, 2015-06-18 17:29:49]

cevans@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2015-06-18 18:00:11]

LGTM

https://codereview.chromium.org/1190173003/diff/1/Source/wtf/PartitionAlloc.cpp
File Source/wtf/PartitionAlloc.cpp (right):

https://codereview.chromium.org/1190173003/diff/1/Source/wtf/PartitionAlloc.c...
Source/wtf/PartitionAlloc.cpp:912: partitionDecommitFreePages(root);

Nit: We might want to rename this function to partitionDecommitEmptyPages.

https://codereview.chromium.org/1190173003/diff/1/Source/wtf/PartitionAlloc.c...
Source/wtf/PartitionAlloc.cpp:1105: statsOut->decommittableBytes +=
pageBytesResidentRounded;

Here we're adding pageBytesResidentRounded to both statsOut->residentBytes and
statsOut->decommittableBytes. Is it an expected thing? I guess it shouldn't be
added to statsOut->residentBytes given that this is an empty page.

[Chris Evans, 2015-06-18 18:10:12]

On 2015/06/18 18:00:11, haraken wrote:
> LGTM
> 
>
https://codereview.chromium.org/1190173003/diff/1/Source/wtf/PartitionAlloc.cpp
> File Source/wtf/PartitionAlloc.cpp (right):
> 
>
https://codereview.chromium.org/1190173003/diff/1/Source/wtf/PartitionAlloc.c...
> Source/wtf/PartitionAlloc.cpp:912: partitionDecommitFreePages(root);
> 
> Nit: We might want to rename this function to partitionDecommitEmptyPages.

Agree, done.

> 
>
https://codereview.chromium.org/1190173003/diff/1/Source/wtf/PartitionAlloc.c...
> Source/wtf/PartitionAlloc.cpp:1105: statsOut->decommittableBytes +=
> pageBytesResidentRounded;
> 
> Here we're adding pageBytesResidentRounded to both statsOut->residentBytes and
> statsOut->decommittableBytes. Is it an expected thing? I guess it shouldn't be
> added to statsOut->residentBytes given that this is an empty page.

I think this is correct. residentBytes is trying to measure the overall physical
memory load of the partition. Memory that is "empty" but still committed counts
as real physical memory usage.

[Chris Evans, 2015-06-18 18:11:37]

The CQ bit was checked by cevans@chromium.org

[Chris Evans, 2015-06-18 18:11:37]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1190173003/#ps20001
(title: "Review feedback.")

[commit-bot: I haz the power, 2015-06-18 18:12:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1190173003/20001

[haraken, 2015-06-18 18:16:55]

On 2015/06/18 18:10:12, Chris Evans wrote:
> On 2015/06/18 18:00:11, haraken wrote:
> > LGTM
> > 
> >
>
https://codereview.chromium.org/1190173003/diff/1/Source/wtf/PartitionAlloc.cpp
> > File Source/wtf/PartitionAlloc.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1190173003/diff/1/Source/wtf/PartitionAlloc.c...
> > Source/wtf/PartitionAlloc.cpp:912: partitionDecommitFreePages(root);
> > 
> > Nit: We might want to rename this function to partitionDecommitEmptyPages.
> 
> Agree, done.
> 
> > 
> >
>
https://codereview.chromium.org/1190173003/diff/1/Source/wtf/PartitionAlloc.c...
> > Source/wtf/PartitionAlloc.cpp:1105: statsOut->decommittableBytes +=
> > pageBytesResidentRounded;
> > 
> > Here we're adding pageBytesResidentRounded to both statsOut->residentBytes
and
> > statsOut->decommittableBytes. Is it an expected thing? I guess it shouldn't
be
> > added to statsOut->residentBytes given that this is an empty page.
> 
> I think this is correct. residentBytes is trying to measure the overall
physical
> memory load of the partition. Memory that is "empty" but still committed
counts
> as real physical memory usage.

ah, that sounds reasonable.

[commit-bot: I haz the power, 2015-06-18 20:28:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-18 20:28:07]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/59573)

[Chris Evans, 2015-06-18 20:29:44]

The CQ bit was checked by cevans@chromium.org

[commit-bot: I haz the power, 2015-06-18 20:30:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1190173003/20001

[commit-bot: I haz the power, 2015-06-18 22:49:57]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197406