Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 1263043007: Add ImmutableHeap class consisting of parts (Closed)

Created:
2 years, 4 months ago by kustermann
Modified:
2 years, 4 months ago
CC:
fletch+reviews_googlegroups.com
Base URL:
git@github.com:dart-lang/fletch.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

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

Patch Set 1 #

Patch Set 2 : #

Total comments: 16

Patch Set 3 : Addressed comments, merged other CL about allocation budget in space #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+189 lines, -2 lines) Patch
M src/vm/heap.h View 1 1 chunk +1 line, -0 lines 0 comments Download
A src/vm/immutable_heap.h View 1 2 1 chunk +79 lines, -0 lines 2 comments Download
A src/vm/immutable_heap.cc View 1 2 1 chunk +100 lines, -0 lines 0 comments Download
M src/vm/object_memory.h View 1 2 3 chunks +4 lines, -2 lines 0 comments Download
M src/vm/object_memory.cc View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M src/vm/vm.gyp View 1 1 chunk +1 line, -0 lines 0 comments Download
Trybot results:

Messages

Total messages: 10 (4 generated)
kustermann
2 years, 4 months ago (2015-08-05 09:21:09 UTC) #3
Mads Ager (google)
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.cc#newcode29 src/vm/immutable_heap.cc:29: CachedHeapPart* cached_part = new CachedHeapPart(); This ...
2 years, 4 months ago (2015-08-05 10:19:03 UTC) #4
kustermann
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.cc#newcode29 src/vm/immutable_heap.cc:29: CachedHeapPart* cached_part = new CachedHeapPart(); On ...
2 years, 4 months ago (2015-08-05 11:11:47 UTC) #6
kustermann
Committed patchset #3 (id:80001) manually as a8a9d4163d77a7561ffae4bf12a143119cf90550 (presubmit successful).
2 years, 4 months ago (2015-08-05 11:14:50 UTC) #7
ricow1
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#newcode59 src/vm/immutable_heap.h:59: Heap* heap_part; why is this called heap_part and not ...
2 years, 4 months ago (2015-08-05 13:18:18 UTC) #9
kustermann
2 years, 4 months ago (2015-08-05 13:20:53 UTC) #10
Message was sent while issue was closed.
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.

Powered by Google App Engine
This is Rietveld 0eb02b776