Refactor skipped code functions interface.

Refactor skipped code functions interface.

Prepares for parallel marking by encapsulating the skipped code functions set, and moving the finalization out to the GCMarker, after all marking is done.

Also changes the interface in the visitor interface to make it less coupled with the representation of the set.

BUG=
R=iposva@google.com

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

[koda, 2015-08-10 16:27:51]

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

[Ivan Posva, 2015-08-27 16:41:30]

I like that we disentangle the actual implementation away from the marker
itself.

-Ivan

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc
File runtime/vm/gc_marker.cc (left):

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc#old...
runtime/vm/gc_marker.cc:174: if (!visit_function_code_) {
Why was this taken out of the marker? Shouldn't this be done in the different
marking threads for their collected code functions once all markers have
finished.

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc
File runtime/vm/gc_marker.cc (right):

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc#new...
runtime/vm/gc_marker.cc:133: GrowableArray<RawFunction*>
skipped_code_functions_;
DISALLOW*

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc#new...
runtime/vm/gc_marker.cc:550: collect_code ? new SkippedCodeFunctions() : NULL;
Why is this not stack or zone allocated?

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc#new...
runtime/vm/gc_marker.cc:561: if (collect_code) {
if (skipped_code_functions != null) {

To protect against a change in the trigger of allocating this object.

[koda, 2015-08-27 17:51:32]

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc
File runtime/vm/gc_marker.cc (left):

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc#old...
runtime/vm/gc_marker.cc:174: if (!visit_function_code_) {
On 2015/08/27 16:41:30, Ivan Posva wrote:
> Why was this taken out of the marker? Shouldn't this be done in the different
> marking threads for their collected code functions once all markers have
> finished.

Moved back, as discussed offline.

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc
File runtime/vm/gc_marker.cc (right):

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc#new...
runtime/vm/gc_marker.cc:133: GrowableArray<RawFunction*>
skipped_code_functions_;
On 2015/08/27 16:41:30, Ivan Posva wrote:
> DISALLOW*

Done.

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc#new...
runtime/vm/gc_marker.cc:550: collect_code ? new SkippedCodeFunctions() : NULL;
On 2015/08/27 16:41:30, Ivan Posva wrote:
> Why is this not stack or zone allocated?

Done.

https://codereview.chromium.org/1287463003/diff/1/runtime/vm/gc_marker.cc#new...
runtime/vm/gc_marker.cc:561: if (collect_code) {
On 2015/08/27 16:41:30, Ivan Posva wrote:
> if (skipped_code_functions != null) {
> 
> To protect against a change in the trigger of allocating this object.

Done.

[Ivan Posva, 2015-08-27 17:58:21]

LGTM -ip

[koda, 2015-08-27 18:19:23]

Committed patchset #5 (id:80001) manually as
51de0c8ff4875386cd05c63cdb1808e57ef589cc (presubmit successful).