Add ImmutableHeap class consisting of parts

Add ImmutableHeap class consisting of parts

The ImmutableHeap has it's own Heap object and a number of parts. Scheduler
worker threads can aquire parts. The parts are used for allocation of immutable
objects while a process is being interpreted. Once an allocation failure
happens, the part is released to the ImmutableHeap.

The ImmutableHeap decides when it should be collected. Currently this is
implemented in a very simple way: As soon as enough parts have been given out
(and returned) it will signal that it should be collected.

The scheduler will request parts, and once it gives a part back to the
ImmutableHeap it might need to trigger a immutable GC.

The GC thread currently stopps the whole program, performs the collection and
resumes the program. But going forward this could happen concurrently with the
scheduler interpreting processes.
[A additional forwarding word for immutable objects and an updated identical()
 implementation should be enough.]

BUG=
R=ager@google.com

Committed: https://github.com/dart-lang/fletch/commit/a8a9d4163d77a7561ffae4bf12a143119cf90550

[kustermann, 2015-08-04 14:48:27]

Patchset #2 (id:20001) has been deleted

[kustermann, 2015-08-05 09:21:08]

kustermann@google.com changed reviewers:
+ ager@google.com

[Mads Ager (google), 2015-08-05 10:19:03]

LGTM with comments.

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.cc
File src/vm/immutable_heap.cc (right):

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.c...
src/vm/immutable_heap.cc:29: CachedHeapPart* cached_part = new CachedHeapPart();
This could be a one-liner?

cached_parts_ = new CachedHeapPart(heap, cached_parts_);

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h
File src/vm/immutable_heap.h (right):

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:19: // ``.
Finish comment please.

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:21: // Will return `true` if the caller should trigger
an immutable GC.
Newline before comment?

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:23: // It is assumed that this function gets only called
on allocation failures.
gets only called -> is called only

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:25: void MergeParts();
Document this one as well?

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:45: struct CachedHeapPart {
Let's just use a class instead of a struct. It is the same modulo visibility.

Why not take heap_part and next in the constructor?

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:51: private:
Remove extra 'private:'

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:61: CachedHeapPart* cached_parts_;
I guess these are the parts that are no longer outstanding because they have
been returned. Maybe call it returned_parts_ or unmerged_parts_? Maybe call
ticks_ number_of_returned_parts_ or number_of_unmerged_parts_?

[kustermann, 2015-08-05 11:10:41]

Patchset #3 (id:60001) has been deleted

[kustermann, 2015-08-05 11:11:47]

Thank you, Mads.

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.cc
File src/vm/immutable_heap.cc (right):

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.c...
src/vm/immutable_heap.cc:29: CachedHeapPart* cached_part = new CachedHeapPart();
On 2015/08/05 10:19:02, Mads Ager (google) wrote:
> This could be a one-liner?
> 
> cached_parts_ = new CachedHeapPart(heap, cached_parts_);

Made it a two liner, see other reason why I don't pass heap in the constructor.

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h
File src/vm/immutable_heap.h (right):

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:19: // ``.
On 2015/08/05 10:19:02, Mads Ager (google) wrote:
> Finish comment please.

Done.

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:21: // Will return `true` if the caller should trigger
an immutable GC.
On 2015/08/05 10:19:02, Mads Ager (google) wrote:
> Newline before comment?

Done.

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:23: // It is assumed that this function gets only called
on allocation failures.
On 2015/08/05 10:19:02, Mads Ager (google) wrote:
> gets only called -> is called only 

Done.

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:25: void MergeParts();
On 2015/08/05 10:19:03, Mads Ager (google) wrote:
> Document this one as well?

Done.

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:45: struct CachedHeapPart {
"Let's just use a class instead of a struct. It is the same modulo visibility."

Done. Now all fields are public and I removed the underscore.

"Why not take heap_part and next in the constructor?"

I've decided that the destructor will not delete the heap. Which means the
object doesn't own the heap. Other code (e.g. Heap, Space) delete what they get
in the constructor, to not make one assume we do the same here, the constructor
doesn't take the heap (or next).

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:51: private:
On 2015/08/05 10:19:03, Mads Ager (google) wrote:
> Remove extra 'private:'

Done.

https://codereview.chromium.org/1263043007/diff/40001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:61: CachedHeapPart* cached_parts_;
On 2015/08/05 10:19:02, Mads Ager (google) wrote:
> I guess these are the parts that are no longer outstanding because they have
> been returned. Maybe call it returned_parts_ or unmerged_parts_? Maybe call
> ticks_ number_of_returned_parts_ or number_of_unmerged_parts_?

Done.

Picked "number_of_unmerged_parts_" and removed/renamed "Cached"  in other
methods and the struct.

[kustermann, 2015-08-05 11:14:50]

Committed patchset #3 (id:80001) manually as
a8a9d4163d77a7561ffae4bf12a143119cf90550 (presubmit successful).

[ricow1, 2015-08-05 13:18:17]

ricow@google.com changed reviewers:
+ ricow@google.com

[ricow1, 2015-08-05 13:18:18]

https://codereview.chromium.org/1263043007/diff/80001/src/vm/immutable_heap.h
File src/vm/immutable_heap.h (right):

https://codereview.chromium.org/1263043007/diff/80001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:59: Heap* heap_part;
why is this called heap_part and not just heap (judging purely by the name I
would assume this is of type HeapPart)

[kustermann, 2015-08-05 13:20:53]

https://codereview.chromium.org/1263043007/diff/80001/src/vm/immutable_heap.h
File src/vm/immutable_heap.h (right):

https://codereview.chromium.org/1263043007/diff/80001/src/vm/immutable_heap.h...
src/vm/immutable_heap.h:59: Heap* heap_part;
On 2015/08/05 13:18:18, ricow1 wrote:
> why is this called heap_part and not just heap (judging purely by the name I
> would assume this is of type HeapPart)

Well, because it is only part of the [ImmutableHeap]. Several parts can be
merged together. The "real merged heap" is on line 70.