HeapCompact: don't cast to BasePage before sanity check.

third_party/WebKit/Source/platform/heap/HeapCompact.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "platform/heap/HeapCompact.h"
 
 #include "platform/Histogram.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/heap/Heap.h"
 #include "platform/heap/SparseHeapBitmap.h"
@@ skipping 65 matching lines @@
     // |refPage|'s arena isn't in the set.
 
     m_fixups.add(reference, slot);
 
     // Note: |slot| will reside outside the Oilpan heap if it is a
     // PersistentHeapCollectionBase. Hence pageFromObject() cannot be
     // used, as it sanity checks the |BasePage| it returns. Simply
     // derive the raw BasePage address here and check if it is a member
     // of the compactable and relocatable page address set.
     Address slotAddress = reinterpret_cast<Address>(slot);
-    BasePage* slotPage = reinterpret_cast<BasePage*>(
-        blinkPageAddress(slotAddress) + blinkGuardPageSize);
-    if (LIKELY(!m_relocatablePages.contains(slotPage)))
+    void* slotPageAddress = blinkPageAddress(slotAddress) + blinkGuardPageSize;
+    if (LIKELY(!m_relocatablePages.contains(slotPageAddress)))
       return;
+    BasePage* slotPage = reinterpret_cast<BasePage*>(slotPageAddress);

[sof, 2017/02/03 16:04:57]

nit: now only used if DCHECK_IS_ON().

 #if DCHECK_IS_ON()
     DCHECK(slotPage->contains(slotAddress));
 #endif
     // Unlikely case, the slot resides on a compacting arena's page.
     //  => It is an 'interior slot' (interior to a movable backing store.)
     // Record it as an interior slot, which entails:
     //
     //  - Storing it in the interior map, which maps the slot to
     //    its (eventual) location. Initially nullptr.
     //  - Mark it as being interior pointer within the page's
@@ skipping 137 matching lines @@
   HashMap<MovableReference, MovableReference*> m_fixups;
 
   // Map from movable reference to callbacks that need to be invoked
   // when the object moves.
   HashMap<MovableReference, std::pair<void*, MovingObjectCallback>>
       m_fixupCallbacks;
 
   // Slot => relocated slot/final location.
   HashMap<MovableReference*, Address> m_interiorFixups;
 
   // All pages that are being compacted.

[sof, 2017/02/02 22:04:41]

Could you update the comment to say that the void* points to the start of a
BasePage? "page" is a bit generic.

[krasin1, 2017/02/02 22:19:09]

Is it better now?

[sof, 2017/02/03 16:04:57]

Much better, thanks :)

-  HashSet<BasePage*> m_relocatablePages;
+  HashSet<void*> m_relocatablePages;
 
   std::unique_ptr<SparseHeapBitmap> m_interiors;
 };
 
 HeapCompact::HeapCompact()
     : m_doCompact(false),
       m_gcCountSinceLastCompaction(0),
       m_freeListSize(0),
       m_compactableArenas(0u),
       m_freedPages(0),
@@ skipping 199 matching lines @@
   fixups().addCompactingPage(page);
 }
 
 bool HeapCompact::scheduleCompactionGCForTesting(bool value) {
   bool current = s_forceCompactionGC;
   s_forceCompactionGC = value;
   return current;
 }
 
 }  // namespace blink