[heap] Decouple SpaceIterator from ObjectIterator.

[heap] Decouple SpaceIterator from ObjectIterator.

BUG=

Committed: https://crrev.com/55dd687a434e667acee4f700398b84a522df7315
Cr-Commit-Position: refs/heads/master@{#39781}

[ulan, 2016-09-27 13:42:49]

ulan@chromium.org changed reviewers:
+ mlippautz@chromium.org

[ulan, 2016-09-27 13:42:50]

ptal

[Michael Lippautz, 2016-09-27 13:53:05]

lgtm

https://codereview.chromium.org/2377513007/diff/1/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/2377513007/diff/1/src/heap/heap.h#newcode2461
src/heap/heap.h:2461: // For each space an object iterator is provided. The
deallocation of the
nit: Outdated comment.

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

https://codereview.chromium.org/2377513007/diff/1/src/heap/spaces.h#newcode2351
src/heap/spaces.h:2351: std::unique_ptr<ObjectIterator> GetObjectIterator()
override;
While you are here, can you move this line and the begin/end iterators above the
#ifdefs? 

(While cleaning up the heap api I made sure that #ifdef DEBUG and VERIFY_HEAP
are the last "sections" in a class decl.)

[ulan, 2016-09-27 14:10:53]

thanks!

https://codereview.chromium.org/2377513007/diff/1/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/2377513007/diff/1/src/heap/heap.h#newcode2461
src/heap/heap.h:2461: // For each space an object iterator is provided. The
deallocation of the
On 2016/09/27 13:53:05, Michael Lippautz wrote:
> nit: Outdated comment.

Done.

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

https://codereview.chromium.org/2377513007/diff/1/src/heap/spaces.h#newcode2351
src/heap/spaces.h:2351: std::unique_ptr<ObjectIterator> GetObjectIterator()
override;
On 2016/09/27 13:53:05, Michael Lippautz wrote:
> While you are here, can you move this line and the begin/end iterators above
the
> #ifdefs? 
> 
> (While cleaning up the heap api I made sure that #ifdef DEBUG and VERIFY_HEAP
> are the last "sections" in a class decl.)

Done.

[ulan, 2016-09-27 14:11:01]

The CQ bit was checked by ulan@chromium.org

[ulan, 2016-09-27 14:11:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlippautz@chromium.org
Link to the patchset: https://codereview.chromium.org/2377513007/#ps40001
(title: "address comments")

[commit-bot: I haz the power, 2016-09-27 14:11:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 15:12:26]

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

[commit-bot: I haz the power, 2016-09-27 15:12:27]

Failed to apply patch for src/heap/spaces.cc:
While running git apply --index -3 -p1;
  error: patch failed: src/heap/spaces.cc:2089
  Falling back to three-way merge...
  Applied patch to 'src/heap/spaces.cc' with conflicts.
  U src/heap/spaces.cc

Patch:       src/heap/spaces.cc
Index: src/heap/spaces.cc
diff --git a/src/heap/spaces.cc b/src/heap/spaces.cc
index
8982a2953273abbe35993b57ab63ee43d42ae0d7..467f0c921602ba13c7e751ceecb1aa4b684ade59
100644
--- a/src/heap/spaces.cc
+++ b/src/heap/spaces.cc
@@ -1430,6 +1430,10 @@ void PagedSpace::ReleasePage(Page* page) {
   heap()->memory_allocator()->Free<MemoryAllocator::kPreFreeAndQueue>(page);
 }
 
+std::unique_ptr<ObjectIterator> PagedSpace::GetObjectIterator() {
+  return std::unique_ptr<ObjectIterator>(new HeapObjectIterator(this));
+}
+
 #ifdef DEBUG
 void PagedSpace::Print() {}
 #endif
@@ -1816,6 +1820,10 @@ void NewSpace::InlineAllocationStep(Address top, Address
new_top,
   }
 }
 
+std::unique_ptr<ObjectIterator> NewSpace::GetObjectIterator() {
+  return std::unique_ptr<ObjectIterator>(new SemiSpaceIterator(this));
+}
+
 #ifdef VERIFY_HEAP
 // We do not use the SemiSpaceIterator because verification doesn't assume
 // that it works (it depends on the invariants we are checking).
@@ -2089,6 +2097,11 @@ void SemiSpace::set_age_mark(Address mark) {
   }
 }
 
+std::unique_ptr<ObjectIterator> SemiSpace::GetObjectIterator() {
+  // Use the NewSpace::NewObjectIterator to iterate the ToSpace.
+  UNREACHABLE();
+  return std::unique_ptr<ObjectIterator>();
+}
 
 #ifdef DEBUG
 void SemiSpace::Print() {}
@@ -3141,6 +3154,9 @@ bool LargeObjectSpace::Contains(HeapObject* object) {
   return owned;
 }
 
+std::unique_ptr<ObjectIterator> LargeObjectSpace::GetObjectIterator() {
+  return std::unique_ptr<ObjectIterator>(new LargeObjectIterator(this));
+}
 
 #ifdef VERIFY_HEAP
 // We do not assume that the large object iterator works, because it depends

[ulan, 2016-09-27 15:42:07]

The CQ bit was checked by ulan@chromium.org

[ulan, 2016-09-27 15:42:08]

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

[commit-bot: I haz the power, 2016-09-27 15:42:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 16:23:39]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-27 16:23:59]

Description was changed from

==========
[heap] Decouple SpaceIterator from ObjectIterator.

BUG=
==========

to

==========
[heap] Decouple SpaceIterator from ObjectIterator.

BUG=

Committed: https://crrev.com/55dd687a434e667acee4f700398b84a522df7315
Cr-Commit-Position: refs/heads/master@{#39781}
==========

[commit-bot: I haz the power, 2016-09-27 16:24:00]

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