I think we already fixed this (but the comment in v8-profiler.h seems out of date -- needs fixing)

mlippautz@: did you notice actual lack of accounting (i.e. have a test case), or did this issue arise from reading that misleading comment?

While reading the source of FreeList::Allocate (https://cs.chromium.org/chromium/src/v8/src/heap/spaces.cc?sq=package:chromium&dr=CSs&rcl=1470208656&l=2452) these days, we noticed that, similar to NewSpace, we set up a linear allocation area. However, unlike with NewSpace, we lack the InlineAllocationStep call. I was under the assumption that the behavior should mirror what's going on in NewSpace, right?

(We removed a double-accounting call to Allocation step in https://codereview.chromium.org/2206623003/. We already have the step calls in bottlenecks for allocation.)

assigning verbally to mattloring@ (who isn't a project member, so I can't actually assign)

The call to AllocationStep inside FreeList::Allocate (from https://codereview.chromium.org/1625753002/) was originally used for this purpose? What was the alternate bottleneck that caused this step to be redundant?

Yes, the call in PagedSpace::AllocateRaw is the bottleneck for all regular PagedSpace allocations (https://cs.chromium.org/chromium/src/v8/src/heap/spaces-inl.h?q=PagedSpace::AllocateRaw&sq=package:chromium&l=506&dr=CSs). We removed the one in FreeList::Allocate because of double steps.

Haven't been following the profiler progress a lot, but shouldn't there be in *Inline*AllocationStep somewhere for PagedSpace?

InlineAllocationStep and AllocationStep are similar mechanisms to track allocations coming from different code paths. As long as all allocations make their way through PagedSpace::AllocateRaw, things should be fine.

The allocations in PagedSpace::AllocateRaw are accounted for, however, we set up a linear allocation area and miss all allocations from generated code.

Gotcha. It seems that we didn't anticipate inline allocation from generated code of anything other than new space. Looking at the code, it seems pre-tenured allocations can also be inline allocated.

It is safe to assume that those are the only two cases that can be inline allocated? For example, I would imagine that it is not possible to inline allocate from LO space.

Yep, we have linear allocation areas for new and old space and don't have them for code and LO space.

I am starting to look at this now (finally!).

Looking at the code, one thing that might help with code-reuse would be if both PagedSpace and NewSpace inherited from a common super class. I want to put the logic of linear allocation into this class (LinearSpace?). I do not think that Space class is the right place for linear allocation stepping logic.

mlippautz@ WDYT?

If that is not agreeable, perhaps I can move the allocation_info_ and linear allocation stepping logic into a mixin class and use multiple inheritance.

I actually thought of introducing something like LinearSpace (no opinion on naming) a while ago. I dropped the idea back then as there was no real need for the abstraction. It looks like this would change now, so yeah, I'd try going this direction.

V8 doesn't make use of mixins and multiple inheritance. We should keep it that way.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/43040781a662b070d498efce0475457834ab8ce3

commit 43040781a662b070d498efce0475457834ab8ce3
Author: Ali Ijaz Sheikh <ofrobots@google.com>
Date: Thu Aug 17 20:06:08 2017

[heap] move GetNextInlineAllocationStepSize to Space

Some preperatory refactoring to allow observation of inline allocations
from Old Space.

BUG= chromium:633920 

Change-Id: Ia1232591860729fcd8942d816aa454171d3aec33
Reviewed-on: https://chromium-review.googlesource.com/617923
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
Cr-Commit-Position: refs/heads/master@{#47413}
[modify] https://crrev.com/43040781a662b070d498efce0475457834ab8ce3/src/heap/heap.h
[modify] https://crrev.com/43040781a662b070d498efce0475457834ab8ce3/src/heap/spaces.cc
[modify] https://crrev.com/43040781a662b070d498efce0475457834ab8ce3/src/heap/spaces.h

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/672a41c3cacc6d6b9a5990ade44c9993d78a556f

commit 672a41c3cacc6d6b9a5990ade44c9993d78a556f
Author: Ali Ijaz Sheikh <ofrobots@google.com>
Date: Fri Sep 15 14:11:46 2017

[profiler] proper observation of old space inline allocations

Bug:  chromium:633920 
Change-Id: I9a2f4a89f6b9c0f63cb3b166b06a88a12f0a203c
Reviewed-on: https://chromium-review.googlesource.com/631696
Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48043}
[modify] https://crrev.com/672a41c3cacc6d6b9a5990ade44c9993d78a556f/src/heap/spaces-inl.h
[modify] https://crrev.com/672a41c3cacc6d6b9a5990ade44c9993d78a556f/src/heap/spaces.cc
[modify] https://crrev.com/672a41c3cacc6d6b9a5990ade44c9993d78a556f/src/heap/spaces.h
[modify] https://crrev.com/672a41c3cacc6d6b9a5990ade44c9993d78a556f/src/profiler/sampling-heap-profiler.h
[modify] https://crrev.com/672a41c3cacc6d6b9a5990ade44c9993d78a556f/test/cctest/heap/heap-utils.cc
[modify] https://crrev.com/672a41c3cacc6d6b9a5990ade44c9993d78a556f/test/cctest/test-heap-profiler.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/258f270f15d25934146405577f7b2bd8a4d3de95

commit 258f270f15d25934146405577f7b2bd8a4d3de95
Author: Ali Ijaz Sheikh <ofrobots@google.com>
Date: Fri Sep 15 20:38:18 2017

Revert "[profiler] proper observation of old space inline allocations"

This reverts commit 672a41c3cacc6d6b9a5990ade44c9993d78a556f.

Reason for revert: Linux64 TSAN bot failures

Original change's description:
> [profiler] proper observation of old space inline allocations
> 
> Bug:  chromium:633920 
> Change-Id: I9a2f4a89f6b9c0f63cb3b166b06a88a12f0a203c
> Reviewed-on: https://chromium-review.googlesource.com/631696
> Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48043}

TBR=ulan@chromium.org,mlippautz@chromium.org,ofrobots@google.com

Change-Id: Ib71baf69b29b067fa0ba76027170054b8faa78d3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:633920 
Reviewed-on: https://chromium-review.googlesource.com/669559
Reviewed-by: Ali Ijaz Sheikh <ofrobots@google.com>
Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
Cr-Commit-Position: refs/heads/master@{#48052}
[modify] https://crrev.com/258f270f15d25934146405577f7b2bd8a4d3de95/src/heap/spaces-inl.h
[modify] https://crrev.com/258f270f15d25934146405577f7b2bd8a4d3de95/src/heap/spaces.cc
[modify] https://crrev.com/258f270f15d25934146405577f7b2bd8a4d3de95/src/heap/spaces.h
[modify] https://crrev.com/258f270f15d25934146405577f7b2bd8a4d3de95/src/profiler/sampling-heap-profiler.h
[modify] https://crrev.com/258f270f15d25934146405577f7b2bd8a4d3de95/test/cctest/heap/heap-utils.cc
[modify] https://crrev.com/258f270f15d25934146405577f7b2bd8a4d3de95/test/cctest/test-heap-profiler.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/ec952aaa68b12e8448c123b5252efa3afa440ceb

commit ec952aaa68b12e8448c123b5252efa3afa440ceb
Author: Ali Ijaz Sheikh <ofrobots@google.com>
Date: Wed Sep 20 01:28:55 2017

Reland "[profiler] proper observation of old space inline allocations"

This is a reland of 672a41c3cacc6d6b9a5990ade44c9993d78a556f
Original change's description:
> [profiler] proper observation of old space inline allocations
> 
> Bug:  chromium:633920 
> Change-Id: I9a2f4a89f6b9c0f63cb3b166b06a88a12f0a203c
> Reviewed-on: https://chromium-review.googlesource.com/631696
> Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48043}

Bug:  chromium:633920 
Change-Id: I6fe743d31b8ff26f3858488d4c014c62d3c85add
Reviewed-on: https://chromium-review.googlesource.com/671127
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
Cr-Commit-Position: refs/heads/master@{#48085}
[modify] https://crrev.com/ec952aaa68b12e8448c123b5252efa3afa440ceb/src/heap/spaces-inl.h
[modify] https://crrev.com/ec952aaa68b12e8448c123b5252efa3afa440ceb/src/heap/spaces.cc
[modify] https://crrev.com/ec952aaa68b12e8448c123b5252efa3afa440ceb/src/heap/spaces.h
[modify] https://crrev.com/ec952aaa68b12e8448c123b5252efa3afa440ceb/src/profiler/sampling-heap-profiler.h
[modify] https://crrev.com/ec952aaa68b12e8448c123b5252efa3afa440ceb/test/cctest/heap/heap-utils.cc
[modify] https://crrev.com/ec952aaa68b12e8448c123b5252efa3afa440ceb/test/cctest/heap/test-invalidated-slots.cc
[modify] https://crrev.com/ec952aaa68b12e8448c123b5252efa3afa440ceb/test/cctest/test-heap-profiler.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/f16b561beeb6eb56c31eda97b81cd1492f41b558

commit f16b561beeb6eb56c31eda97b81cd1492f41b558
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Thu Sep 21 04:19:29 2017

Revert "Reland "[profiler] proper observation of old space inline allocations""

This reverts commit ec952aaa68b12e8448c123b5252efa3afa440ceb.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=767144

Original change's description:
> Reland "[profiler] proper observation of old space inline allocations"
> 
> This is a reland of 672a41c3cacc6d6b9a5990ade44c9993d78a556f
> Original change's description:
> > [profiler] proper observation of old space inline allocations
> > 
> > Bug:  chromium:633920 
> > Change-Id: I9a2f4a89f6b9c0f63cb3b166b06a88a12f0a203c
> > Reviewed-on: https://chromium-review.googlesource.com/631696
> > Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#48043}
> 
> Bug:  chromium:633920 
> Change-Id: I6fe743d31b8ff26f3858488d4c014c62d3c85add
> Reviewed-on: https://chromium-review.googlesource.com/671127
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
> Cr-Commit-Position: refs/heads/master@{#48085}

TBR=ulan@chromium.org,mlippautz@chromium.org,ofrobots@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  chromium:633920 
Change-Id: I576cdab4a03f9fe057ebe1bf7da9dfe3c7bf62cd
Reviewed-on: https://chromium-review.googlesource.com/676683
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48098}
[modify] https://crrev.com/f16b561beeb6eb56c31eda97b81cd1492f41b558/src/heap/spaces-inl.h
[modify] https://crrev.com/f16b561beeb6eb56c31eda97b81cd1492f41b558/src/heap/spaces.cc
[modify] https://crrev.com/f16b561beeb6eb56c31eda97b81cd1492f41b558/src/heap/spaces.h
[modify] https://crrev.com/f16b561beeb6eb56c31eda97b81cd1492f41b558/src/profiler/sampling-heap-profiler.h
[modify] https://crrev.com/f16b561beeb6eb56c31eda97b81cd1492f41b558/test/cctest/heap/heap-utils.cc
[modify] https://crrev.com/f16b561beeb6eb56c31eda97b81cd1492f41b558/test/cctest/heap/test-invalidated-slots.cc
[modify] https://crrev.com/f16b561beeb6eb56c31eda97b81cd1492f41b558/test/cctest/test-heap-profiler.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/52e8d0ab40fdddfd0789ef911f7c25fc8405ca91

commit 52e8d0ab40fdddfd0789ef911f7c25fc8405ca91
Author: Ali Ijaz Sheikh <ofrobots@google.com>
Date: Mon Sep 25 15:13:01 2017

Reland "Reland "[profiler] proper observation of old space inline allocations""

This is a reland of ec952aaa68b12e8448c123b5252efa3afa440ceb.

Included is a fix that ensures that top_on_previous_step_ is cleared when we
release a page.

Original change's description:
> Reland "[profiler] proper observation of old space inline allocations"
>
> This is a reland of 672a41c3cacc6d6b9a5990ade44c9993d78a556f
> Original change's description:
> > [profiler] proper observation of old space inline allocations
> >
> > Bug:  chromium:633920 
> > Change-Id: I9a2f4a89f6b9c0f63cb3b166b06a88a12f0a203c
> > Reviewed-on: https://chromium-review.googlesource.com/631696
> > Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#48043}
>
> Bug:  chromium:633920 
> Change-Id: I6fe743d31b8ff26f3858488d4c014c62d3c85add
> Reviewed-on: https://chromium-review.googlesource.com/671127
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
> Cr-Commit-Position: refs/heads/master@{#48085}

Bug:  chromium:633920 
Change-Id: I8a0dcc4eaffc1f1d3ac5b3f8d344001cdae36606
Reviewed-on: https://chromium-review.googlesource.com/677407
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
Cr-Commit-Position: refs/heads/master@{#48141}
[modify] https://crrev.com/52e8d0ab40fdddfd0789ef911f7c25fc8405ca91/src/heap/spaces-inl.h
[modify] https://crrev.com/52e8d0ab40fdddfd0789ef911f7c25fc8405ca91/src/heap/spaces.cc
[modify] https://crrev.com/52e8d0ab40fdddfd0789ef911f7c25fc8405ca91/src/heap/spaces.h
[modify] https://crrev.com/52e8d0ab40fdddfd0789ef911f7c25fc8405ca91/src/profiler/sampling-heap-profiler.h
[modify] https://crrev.com/52e8d0ab40fdddfd0789ef911f7c25fc8405ca91/test/cctest/heap/heap-utils.cc
[modify] https://crrev.com/52e8d0ab40fdddfd0789ef911f7c25fc8405ca91/test/cctest/heap/test-invalidated-slots.cc
[modify] https://crrev.com/52e8d0ab40fdddfd0789ef911f7c25fc8405ca91/test/cctest/test-heap-profiler.cc

I believe this is fixed now. There is some refactoring and improvements to follow, but old space linear allocation accounting is in place now.