Distinct block sizes for StoreBuffer/MarkingStack.

Distinct block sizes for StoreBuffer/MarkingStack.

Remove typedef hack by templatizing the shared structures,
parameterized by block size.

Smaller marking stack blocks are needed for better work distribution during parallel GC.

As a consequence, MarkingStack and StoreBuffer have separate global caches of empty blocks.

Also, use inheritance to differentiate: only StoreBuffer needs the threshold-checking functionality.

BUG=
R=johnmccutchan@google.com

Committed: https://github.com/dart-lang/sdk/commit/e98fcf935426451edc1b36ee7e3ae4ba5d4e4647

[koda, 2015-09-17 17:01:38]

koda@google.com changed reviewers:
+ johnmccutchan@google.com

[koda, 2015-09-17 17:24:15]

Note that this will not have any performance benefit until multiple marking
tasks are actually running in parallel.

(Running benchmarks now to verify it's currently performance neutral.)

[Cutch, 2015-09-17 18:04:06]

LGTM with one comment and one suggestion

https://codereview.chromium.org/1350933004/diff/40001/runtime/vm/store_buffer.h
File runtime/vm/store_buffer.h (right):

https://codereview.chromium.org/1350933004/diff/40001/runtime/vm/store_buffer...
runtime/vm/store_buffer.h:130: static Mutex* global_mutex_;
Will it always be true that blocks of the same size also share the same list and
mutex? That is, will we ever need two distinct lists of PointerBlock<100>? This
assumption should be documented somewhere too.

https://codereview.chromium.org/1350933004/diff/40001/runtime/vm/store_buffer...
runtime/vm/store_buffer.h:146: void PushBlock(Block* block, bool
check_threshold);
Instead of bool check_threshold could we have something like:

enum {
  kCheckThreshold,
  kIgnoreThreshold
}

Makes the code passing ", false" easier to understand when you see ",
StoreBuffer::kIgnoreThreshold"

[koda, 2015-09-17 19:13:18]

https://codereview.chromium.org/1350933004/diff/40001/runtime/vm/store_buffer.h
File runtime/vm/store_buffer.h (right):

https://codereview.chromium.org/1350933004/diff/40001/runtime/vm/store_buffer...
runtime/vm/store_buffer.h:130: static Mutex* global_mutex_;
On 2015/09/17 18:04:06, Cutch wrote:
> Will it always be true that blocks of the same size also share the same list
and
> mutex? That is, will we ever need two distinct lists of PointerBlock<100>?
This
> assumption should be documented somewhere too.

Yes, the cache of empty blocks will be shared if we ever have two types with
matching block size, which I think is most likely what we want: being empty,
they are perfectly shareable, and if lock contention is an issue, we should
probably just add per-thread caches (still based on block size) rather than base
it on the type.

But a good idea to document it; added comments to class.

https://codereview.chromium.org/1350933004/diff/40001/runtime/vm/store_buffer...
runtime/vm/store_buffer.h:146: void PushBlock(Block* block, bool
check_threshold);
On 2015/09/17 18:04:05, Cutch wrote:
> Instead of bool check_threshold could we have something like:
> 
> enum {
>   kCheckThreshold,
>   kIgnoreThreshold
> }
> 
> Makes the code passing ", false" easier to understand when you see ",
> StoreBuffer::kIgnoreThreshold"

Done.

[koda, 2015-09-17 19:22:00]

Committed patchset #4 (id:60001) manually as
e98fcf935426451edc1b36ee7e3ae4ba5d4e4647 (presubmit successful).