Oilpan: Simplify Heap::allocate

Source/platform/heap/Heap.h

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 234 matching lines @@
     size_t payloadSize() { return heapObjectHeader()->payloadSize(); }
 
     Header* heapObjectHeader()
     {
         Address headerAddress = address() + sizeof(LargeHeapObject<Header>) + headerPadding<Header>();
         return reinterpret_cast<Header*>(headerAddress);
     }
 
     bool isMarked();
     void unmark();
-    void getStats(HeapStats&);
+    void getStatsForTesting(HeapStats&);
     void mark(Visitor*);
     void finalize();
     void setDeadMark();
     virtual void markOrphaned()
     {
         // Zap the payload with a recognizable value to detect any incorrect
         // cross thread pointer usage.
         memset(payload(), orphanedZapValue, payloadSize());
         BaseHeapPage::markOrphaned();
     }
@@ skipping 250 matching lines @@
         return address() + sizeof(*this) + headerPadding<Header>();
     }
 
     static size_t payloadSize()
     {
         return (blinkPagePayloadSize() - sizeof(HeapPage) - headerPadding<Header>()) & ~allocationMask;
     }
 
     Address end() { return payload() + payloadSize(); }
 
-    void getStats(HeapStats&);
+    void getStatsForTesting(HeapStats&);
     void clearLiveAndMarkDead();
     void sweep(HeapStats*, ThreadHeap<Header>*);
     void clearObjectStartBitMap();
     void finalize(Header*);
     virtual void checkAndMarkPointer(Visitor*, Address) override;
 #if ENABLE(GC_PROFILE_MARKING)
     const GCInfo* findGCInfo(Address) override;
 #endif
 #if ENABLE(GC_PROFILE_HEAP)
     virtual void snapshot(TracedValue*, ThreadState::SnapshotInfo*);
@@ skipping 155 matching lines @@
 
     // Sweep this part of the Blink heap. This finalizes dead objects
     // and builds freelists for all the unused memory.
     virtual void sweep(HeapStats*) = 0;
     virtual void postSweepProcessing() = 0;
 
     virtual void clearFreeLists() = 0;
     virtual void clearLiveAndMarkDead() = 0;
 
     virtual void makeConsistentForSweeping() = 0;
-
 #if ENABLE(ASSERT)
     virtual bool isConsistentForSweeping() = 0;
+#endif
+    virtual void getStatsForTesting(HeapStats&) = 0;
 
-    virtual void getScannedStats(HeapStats&) = 0;
-#endif
+    virtual void updateRemainingAllocationSize() = 0;
 
     virtual void prepareHeapForTermination() = 0;
 
     virtual int normalPageCount() = 0;
 
     virtual BaseHeap* split(int normalPages) = 0;
     virtual void merge(BaseHeap* other) = 0;
 
     // Returns a bucket number for inserting a FreeListEntry of a
     // given size. All FreeListEntries in the given bucket, n, have
@@ skipping 26 matching lines @@
     virtual void snapshot(TracedValue*, ThreadState::SnapshotInfo*);
 #endif
 
     virtual void sweep(HeapStats*);
     virtual void postSweepProcessing();
 
     virtual void clearFreeLists();
     virtual void clearLiveAndMarkDead();
 
     virtual void makeConsistentForSweeping();
-
 #if ENABLE(ASSERT)
     virtual bool isConsistentForSweeping();
+#endif
+    virtual void getStatsForTesting(HeapStats&);
 
-    virtual void getScannedStats(HeapStats&);
-#endif
+    virtual void updateRemainingAllocationSize();
 
     ThreadState* threadState() { return m_threadState; }
     HeapStats& stats() { return m_threadState->stats(); }
 
     inline Address allocate(size_t, const GCInfo*);
     void addToFreeList(Address, size_t);
     inline static size_t roundedAllocationSize(size_t size)
     {
         return allocationSizeFromSize(size) - sizeof(Header);
     }
 
     virtual void prepareHeapForTermination();
 
     virtual int normalPageCount() { return m_numberOfNormalPages; }
 
     virtual BaseHeap* split(int numberOfNormalPages);
     virtual void merge(BaseHeap* splitOffBase);
 
     void removePageFromHeap(HeapPage<Header>*);
 
     PLATFORM_EXPORT void promptlyFreeObject(Header*);
 
 private:
     void addPageToHeap(const GCInfo*);
     PLATFORM_EXPORT Address outOfLineAllocate(size_t, const GCInfo*);
     static size_t allocationSizeFromSize(size_t);
     PLATFORM_EXPORT Address allocateLargeObject(size_t, const GCInfo*);
     Address currentAllocationPoint() const { return m_currentAllocationPoint; }
-    size_t remainingAllocationSize() const { return m_remainingAllocationSize; }
+    size_t remainingAllocationSize() const
+    {
+        RELEASE_ASSERT(m_allocationLimit >= m_currentAllocationPoint);
+        return static_cast<size_t>(m_allocationLimit - m_currentAllocationPoint);
+    }
     bool ownsNonEmptyAllocationArea() const { return currentAllocationPoint() && remainingAllocationSize(); }
     void setAllocationPoint(Address point, size_t size)
     {
         ASSERT(!point || heapPageFromAddress(point));
         ASSERT(size <= HeapPage<Header>::payloadSize());
+        updateRemainingAllocationSize();
         m_currentAllocationPoint = point;
-        m_remainingAllocationSize = size;
+        m_allocationLimit = point + size;
+        m_lastRemainingAllocationSize = remainingAllocationSize();
     }
     void ensureCurrentAllocation(size_t, const GCInfo*);
     bool allocateFromFreeList(size_t);
 
     void freeLargeObject(LargeHeapObject<Header>*, LargeHeapObject<Header>**);
     void allocatePage(const GCInfo*);
 
 #if ENABLE(ASSERT)
     bool pagesToBeSweptContains(Address);
     bool pagesAllocatedDuringSweepingContains(Address);
 #endif
 
     void sweepNormalPages(HeapStats*);
     void sweepLargePages(HeapStats*);
     bool coalesce(size_t);
 
     Address m_currentAllocationPoint;
-    size_t m_remainingAllocationSize;
+    Address m_allocationLimit;
+    size_t m_lastRemainingAllocationSize;
 
     HeapPage<Header>* m_firstPage;
     LargeHeapObject<Header>* m_firstLargeHeapObject;
 
     HeapPage<Header>* m_firstPageAllocatedDuringSweeping;
     HeapPage<Header>* m_lastPageAllocatedDuringSweeping;
 
     // Merge point for parallel sweep.
     HeapPage<Header>* m_mergePoint;
 
@@ skipping 104 matching lines @@
     static const GCInfo* findGCInfo(Address);
 
     static String createBacktraceString();
 #endif
 
     // Collect heap stats for all threads attached to the Blink
     // garbage collector. Should only be called during garbage
     // collection where threads are known to be at safe points.
     static void getStats(HeapStats*);
 
+    static void getStatsForTesting(HeapStats*);
+
     static void getHeapSpaceSize(uint64_t*, uint64_t*);
 
     static void makeConsistentForSweeping();
 
 #if ENABLE(ASSERT)
     static bool isConsistentForSweeping();
 #endif
 
     static void flushHeapDoesNotContainCache();
 
@@ skipping 396 matching lines @@
     size_t allocationSize = size + sizeof(Header);
     // Align size with allocation granularity.
     allocationSize = (allocationSize + allocationMask) & ~allocationMask;
     return allocationSize;
 }
 
 template<typename Header>
 Address ThreadHeap<Header>::allocate(size_t size, const GCInfo* gcInfo)
 {
     size_t allocationSize = allocationSizeFromSize(size);
-    bool isLargeObject = allocationSize > blinkPageSize / 2;
-    if (isLargeObject)
-        return allocateLargeObject(allocationSize, gcInfo);
-    if (m_remainingAllocationSize < allocationSize)
-        return outOfLineAllocate(size, gcInfo);
-    Address headerAddress = m_currentAllocationPoint;
-    m_currentAllocationPoint += allocationSize;
-    m_remainingAllocationSize -= allocationSize;
-    Header* header = new (NotNull, headerAddress) Header(allocationSize, gcInfo);
-    size_t payloadSize = allocationSize - sizeof(Header);
-    stats().increaseObjectSpace(payloadSize);
-    Address result = headerAddress + sizeof(*header);
-    ASSERT(!(reinterpret_cast<uintptr_t>(result) & allocationMask));
-    // Unpoison the memory used for the object (payload).
-    ASAN_UNPOISON_MEMORY_REGION(result, payloadSize);
+    Address nextAllocationPoint = m_currentAllocationPoint + allocationSize;
+    if (LIKELY(nextAllocationPoint <= m_allocationLimit)) {

[Erik Corry, 2014/10/23 07:18:36]

I'm not sure this change is OK.  allocationSize can be very large, up to
maxHeapObjectSize.  There could be a heap page close to the top of the memory
space, and the nextAllocation Point would then wrap around to the start of
memory and this "if" would erroneously be true.

It was the intention of the old code that this allocate function would often be
inlined with a constant "size", in which case the "if (largeObject)" would
disappear after constant propagation.  I don't know whether the inliner is eager
enough for this in practice.

I personally feel that this function is prettier with an early "if (...) return
..." instead of having everything in an "if" like this.  That's just taste
though, it's up to you.

[haraken, 2014/10/23 08:37:53]

Done.
I'd prefer the new style since it removes # of branches from the fast path,
although that didn't affect performance :)

+        Address headerAddress = m_currentAllocationPoint;
+        m_currentAllocationPoint = nextAllocationPoint;
+        Header* header = new (NotNull, headerAddress) Header(allocationSize, gcInfo);
+        Address result = headerAddress + sizeof(*header);
+        ASSERT(!(reinterpret_cast<uintptr_t>(result) & allocationMask));
+
+        // Unpoison the memory used for the object (payload).
+        ASAN_UNPOISON_MEMORY_REGION(result, allocationSize - sizeof(Header));
 #if ENABLE(ASSERT) || defined(LEAK_SANITIZER) || defined(ADDRESS_SANITIZER)
-    memset(result, 0, payloadSize);
+        memset(result, 0, allocationSize - sizeof(Header));
 #endif
-    ASSERT(heapPageFromAddress(headerAddress + allocationSize - 1));
-    return result;
+        ASSERT(heapPageFromAddress(headerAddress + allocationSize - 1));
+        return result;
+    }
+    return outOfLineAllocate(size, gcInfo);
 }
 
 template<typename T, typename HeapTraits>
 Address Heap::allocate(size_t size)
 {
     ThreadState* state = ThreadStateFor<ThreadingTrait<T>::Affinity>::state();
     ASSERT(state->isAllocationAllowed());
     const GCInfo* gcInfo = GCInfoTrait<T>::get();
     int heapIndex = HeapTraits::index(gcInfo->hasFinalizer());
     BaseHeap* heap = state->heap(heapIndex);
@@ skipping 1001 matching lines @@
 };
 
 template<typename T>
 struct IfWeakMember<WeakMember<T> > {
     static bool isDead(Visitor* visitor, const WeakMember<T>& t) { return !visitor->isAlive(t.get()); }
 };
 
 }
 
 #endif // Heap_h