Oilpan: always shrink tail-allocated backing storage (reland.)

Oilpan: always shrink tail-allocated backing storage (reland.)

When shrinking backing storage allocations, some care is taken not to
introduce excessive fragmentation and/or limit later expansion of the
same object. That logic does not need to apply if the backing store is
at the tail of the current heap chunk: unconditionally reset the
allocation point, if it is.

(Reland of r353321; it asserted on empty shrink attempts on 32-bit windows.)

R=haraken
BUG=420515
Committed: https://crrev.com/73fe1f117f1cc427fa987d7b572fe42b126707e0
Cr-Commit-Position: refs/heads/master@{#354474}

[sof, 2015-10-09 14:18:19]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[sof, 2015-10-09 14:18:19]

please take a look.

Initial blink_perf results looks about the same, but need more runs to gain
confidence wrt noise.

[haraken, 2015-10-09 14:21:41]

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

[haraken, 2015-10-09 14:21:42]

LGTM

https://codereview.chromium.org/1393863004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/heap/HeapPage.cpp (right):

https://codereview.chromium.org/1393863004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/HeapPage.cpp:702: if
(isObjectAllocatedLast(header) && expandSize <= m_remainingAllocationSize) {

isObjectAllocatedAtAllocationPoint ?

[sof, 2015-10-09 14:26:40]

https://codereview.chromium.org/1393863004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/heap/HeapPage.cpp (right):

https://codereview.chromium.org/1393863004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/HeapPage.cpp:702: if
(isObjectAllocatedLast(header) && expandSize <= m_remainingAllocationSize) {
On 2015/10/09 14:21:41, haraken wrote:
> 
> isObjectAllocatedAtAllocationPoint ?

A bit long, but why not; done.

[sof, 2015-10-09 17:31:51]

On 2015/10/09 14:18:19, sof wrote:
> please take a look.
> 
> Initial blink_perf results looks about the same, but need more runs to gain
> confidence wrt noise.

No regressions seen, but the halfa dozen 5-10% gains seen are for tests that
have been known to be noisy, so I won't claim them as improving unless someone
can reproduce :)

Anyway, I'm confident enough that this little improvement is safe to land.

[sof, 2015-10-09 17:32:08]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-10-09 17:32:09]

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/1393863004/#ps20001
(title: "rename predicate")

[commit-bot: I haz the power, 2015-10-09 17:33:03]

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

[commit-bot: I haz the power, 2015-10-09 17:39:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-09 17:40:22]

Patchset 2 (id:??) landed as
https://crrev.com/afa3e61596ebb5d648a6e9923de85534926d27fa
Cr-Commit-Position: refs/heads/master@{#353321}

[Yuta Kitamura, 2015-10-15 09:54:42]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1395493008/ by yutak@chromium.org.

The reason for reverting is: Suspect that this change has caused assertion
failures on
Oilpan+Windows+Debug bot.

http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win%20Oilpan%20...
(Builds are purple because they quit early due to
 too many crashes.)

Crash logs look like:
02:49:04.570 1008   ASSERTION FAILED: header->payloadSize() > newSize
02:49:04.570 1008   Backtrace:
02:49:04.570 1008   	blink::NormalPageHeap::shrinkObject [0x084A1504+148]
02:49:04.570 1008   	blink::HeapAllocator::backingShrink [0x0849AE8A+394]
02:49:04.570 1008   	blink::HeapAllocator::shrinkVectorBacking [0x0849B064+20]
02:49:04.570 1008  
	WTF::VectorBuffer<blink::Member<blink::Node>,0,blink::HeapAllocator>::shrinkBuffer
[0x189E87A6+134]
02:49:04.570 1008  
	WTF::Vector<blink::Member<blink::Node>,0,blink::HeapAllocator>::shrinkCapacity
[0x189E8BA6+102]
02:49:04.570 1008  
	WTF::Vector<blink::Member<blink::Node>,0,blink::HeapAllocator>::shrinkToFit
[0x19A6E65F+31]
02:49:04.570 1008   	blink::DistributedNodes::shrinkToFit [0x19A6E686+22]
(snip).

[sof, 2015-10-16 07:07:24]

Re-opened this one so as to address the case where vector backing storage shrink
attempts end up mapping to the same quantized size as the current size. ptal?

i.e., HeapAllocator::backingShrink() now includes the check that
DefaultAllocator::shrinkVectorBacking() already has.

[haraken, 2015-10-16 07:07:58]

LGTM

[sof, 2015-10-16 07:34:10]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-10-16 07:35:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1393863004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1393863004/40001

[commit-bot: I haz the power, 2015-10-16 08:36:49]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-16 08:37:47]

Patchset 3 (id:??) landed as
https://crrev.com/73fe1f117f1cc427fa987d7b572fe42b126707e0
Cr-Commit-Position: refs/heads/master@{#354474}