Allocation sampling for paged/lo spaces

src/heap/heap.h

Index: src/heap/heap.h
diff --git a/src/heap/heap.h b/src/heap/heap.h
index 3c17dc7d9fbfca808679178857ca57e3e1fa96f4..70e5744d9307f020756efa88be6d5ed724d62ec2 100644
--- a/src/heap/heap.h
+++ b/src/heap/heap.h
@@ -470,6 +470,7 @@ namespace internal {
   PRIVATE_SYMBOL_LIST(V)
 
 // Forward declarations.
+class AllocationObserver;
 class ArrayBufferTracker;
 class GCIdleTimeAction;
 class GCIdleTimeHandler;
@@ -478,7 +479,6 @@ class GCTracer;
 class HeapObjectsFilter;
 class HeapStats;
 class HistogramTimer;
-class InlineAllocationObserver;
 class Isolate;
 class MemoryReducer;
 class ObjectStats;
@@ -2342,7 +2342,7 @@ class Heap {
 
   ScavengeJob* scavenge_job_;
 
-  InlineAllocationObserver* idle_scavenge_observer_;
+  AllocationObserver* idle_scavenge_observer_;
 
   // These two counters are monotomically increasing and never reset.
   size_t full_codegen_bytes_generated_;
@@ -2795,20 +2795,20 @@ class PathTracer : public ObjectVisitor {
 #endif  // DEBUG
 
 // -----------------------------------------------------------------------------
-// Allows observation of inline allocation in the new space.
-class InlineAllocationObserver {
+// Allows observation of allocations.
+class AllocationObserver {
  public:
-  explicit InlineAllocationObserver(intptr_t step_size)
-      : step_size_(step_size), bytes_to_next_step_(step_size) {
+  explicit AllocationObserver(Heap* heap, intptr_t step_size)

[ofrobots, 2016/01/23 16:16:30]

'explicit' is only needed on single argument constructors.

[mattloring, 2016/01/26 00:42:48]

Done.

+      : heap_(heap), step_size_(step_size), bytes_to_next_step_(step_size) {
     DCHECK(step_size >= kPointerSize);
   }
-  virtual ~InlineAllocationObserver() {}
+  virtual ~AllocationObserver() {}
 
   // Called each time the new space does an inline allocation step. This may be

[ofrobots, 2016/01/23 16:16:30]

Update comment since it applies to more than just new space now.

[mattloring, 2016/01/26 00:42:48]

Done.

   // more frequently than the step_size we are monitoring (e.g. when there are
   // multiple observers, or when page or space boundary is encountered.)
-  void InlineAllocationStep(int bytes_allocated, Address soon_object,
-                            size_t size) {
+  void AllocationStep(int bytes_allocated, Address soon_object, size_t size) {
+    if (heap_->gc_state() != Heap::NOT_IN_GC) return;

[ofrobots, 2016/01/23 16:16:31]

It would be better to generalize the PauseInlineAllocationObserver concept. The
AllocationStep is supposed to cover >= 1 allocations, and checking the *current*
heap state sounds wrong when multiple allocations are being reported in
aggregate.

[mattloring, 2016/01/26 00:42:48]

I've generalized it and pausing is unified across all spaces. It would be nice
to find a better way to isolate the locations where pause checks must be
performed. Right now, checks are made before each call to AllocationStep by
spaces other than new_space which seems analogous to what we had before.
@ofrobots, how do you envision these checks being performed to allow for
multiple allocations to aggregate?

[ofrobots, 2016/01/26 15:47:15]

Looking at the way this works, it seems like non-New spaces always call this on
single objects, so this will never be an aggregate for those spaces. If
PauseAlloctionObservers is working correctly, the check would not be necessary
(and could instead be assertions).

     bytes_to_next_step_ -= bytes_allocated;
     if (bytes_to_next_step_ <= 0) {
       Step(static_cast<int>(step_size_ - bytes_to_next_step_), soon_object,
@@ -2839,12 +2839,15 @@ class InlineAllocationObserver {
   // Subclasses can override this method to make step size dynamic.
   virtual intptr_t GetNextStepSize() { return step_size_; }
 
+  Heap* heap_;
   intptr_t step_size_;
   intptr_t bytes_to_next_step_;
 
  private:
+  friend class LargeObjectSpace;
   friend class NewSpace;
-  DISALLOW_COPY_AND_ASSIGN(InlineAllocationObserver);
+  friend class PagedSpace;
+  DISALLOW_COPY_AND_ASSIGN(AllocationObserver);
 };
 
 }  // namespace internal