[heap] make inline allocation step size dynamic

[heap] make inline allocation step size dynamic

Presently the inline allocation step is a static value defined to be the minimum
of the step sizes over all the observers. The step occur every (approx.) step
byte. This is unfair to observers whose steps are not evenly divisible by the
min step size. For example, consider two observers with steps sizes of 512 and
576 bytes. Across 16kb allocated, you would expect the first observer to be hit
approximately 32 times, and the second observer to be hit approximately 28
times.

In reality, the observers get notified 30 and 15 times respectively. The reason
is that each step is 512 bytes, and since 576 is not evenly divisible by 512,
it gets notified much less frequently.

This CL fixes the problem by making the next step size be the minimum (over all
observers) of the remaining bytes to get to the step, making the steps fair.

BUG=
R=hpayer@chromium.org,ulan@chromium.org

Committed: https://crrev.com/f5836617843cb4749c82954d0788f64736803509
Cr-Commit-Position: refs/heads/master@{#31948}

[Hannes Payer (out of office), 2015-11-09 19:26:39]

lgtm

https://codereview.chromium.org/1427973006/diff/1/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1427973006/diff/1/src/heap/spaces.cc#newcode1521
src/heap/spaces.cc:1521: for (int i = 0; i <
inline_allocation_observers_.length(); ++i) {
Can we move that code into a helper function?

https://codereview.chromium.org/1427973006/diff/1/src/heap/spaces.cc#newcode1613
src/heap/spaces.cc:1613: top_on_previous_step_ =
This method is now just updating top_on_previous_step_. We should rename this
method to reflect that.

[ofrobots, 2015-11-09 23:00:32]

https://codereview.chromium.org/1427973006/diff/1/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1427973006/diff/1/src/heap/spaces.cc#newcode1521
src/heap/spaces.cc:1521: for (int i = 0; i <
inline_allocation_observers_.length(); ++i) {
On 2015/11/09 19:26:39, Hannes Payer wrote:
> Can we move that code into a helper function?

Done.

https://codereview.chromium.org/1427973006/diff/1/src/heap/spaces.cc#newcode1613
src/heap/spaces.cc:1613: top_on_previous_step_ =
On 2015/11/09 19:26:39, Hannes Payer wrote:
> This method is now just updating top_on_previous_step_. We should rename this
> method to reflect that.

Done.

[Hannes Payer (out of office), 2015-11-10 17:36:24]

https://codereview.chromium.org/1427973006/diff/20001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1427973006/diff/20001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:1618: DCHECK(next_step != 0);
If no observers are registered, the next_step may be 0. We should check in the
DCHECK if observers exist.

[ofrobots, 2015-11-11 01:46:11]

https://codereview.chromium.org/1427973006/diff/20001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1427973006/diff/20001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:1618: DCHECK(next_step != 0);
On 2015/11/10 17:36:24, Hannes Payer wrote:
> If no observers are registered, the next_step may be 0. We should check in the
> DCHECK if observers exist.

Done.

[Hannes Payer (out of office), 2015-11-11 18:55:54]

lgtm

[ofrobots, 2015-11-11 19:14:45]

The CQ bit was checked by ofrobots@google.com

[commit-bot: I haz the power, 2015-11-11 19:14:48]

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

[commit-bot: I haz the power, 2015-11-11 19:15:48]

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

[commit-bot: I haz the power, 2015-11-11 19:15:48]

Try jobs failed on following builders:
  v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/11741)

[ofrobots, 2015-11-11 20:23:22]

The CQ bit was checked by ofrobots@google.com

[ofrobots, 2015-11-11 20:23:22]

The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org
Link to the patchset: https://codereview.chromium.org/1427973006/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2015-11-11 20:23:32]

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

[commit-bot: I haz the power, 2015-11-11 20:55:27]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-14 23:20:15]

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