1437602 - Refactor gc/Zone.h a little

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P3)

Description

[Jon Coppeard (:jonco)]

Attachment: zone-1-split-out-iterators

Patch to split out iterators from Zone.h.  Previously this contained the iterators for zone groups, zones and compartments.  This moves them to a new gc/Iteration.h header.

I renamed the existing gc/Iteration-inl.h header to gc/GCIteration.h.  This file contains internal iterators used mainly from the GC itself.

It turned out that the AutoPrepareForTracing constructor took a ZoneSelector argument that it didn't use so I removed that.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: zone-2-split-out-scheduling

This patch moves all the scheduling-related structures to a new header, gc/Scheduling.h.  I also moved the huge comment describing this, but I didn't check it's still correct... this could do with a thorough review.

Comment 2

[Paul Bone [:pbone]]

Comment on attachment 8950284 [details] [diff] [review]
zone-1-split-out-iterators

Review of attachment 8950284 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't check for changes to the moved code itself.

If there are no changes to the ieterators themselves then this is r+ except for two minor issues.

Thanks.

::: js/src/shell/js.cpp
@@ +79,5 @@
>  #endif // defined(JS_BUILD_BINAST)
>  
>  #include "frontend/Parser.h"
> +#include "gc/Iteration.h"
> +//#include "gc/GCInternals.h"

Should this commented out line be deleted?

::: js/src/vm/SelfHosting.cpp
@@ +39,5 @@
>  #include "builtin/SIMD.h"
>  #include "builtin/Stream.h"
>  #include "builtin/TypedObject.h"
>  #include "builtin/WeakMapObject.h"
> +//#include "gc/GCIteration.h"

And here.

Comment 3

[Paul Bone [:pbone]]

Comment on attachment 8950286 [details] [diff] [review]
zone-2-split-out-scheduling

Review of attachment 8950286 [details] [diff] [review]:
-----------------------------------------------------------------

Again, I'm assuming there's no change to the moved code itself.

Thanks for doing this r+.

Comment 4

[Jon Coppeard (:jonco)]

(In reply to Paul Bone [:pbone] from comment #2)
> Should this commented out line be deleted?

Ugh, yes, I forgot to take those out.

Cheers!

Comment 5

[Jon Coppeard (:jonco)]

Attachment: zone-1-split-out-iterators v2

I had to rename GCIterators.h to GCIterators-inl.h because our style checker complained about it including jsgcincludes.h.  Carrying r+.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/40bbf952db46
Split out zone and compartment iterators from gc/Zone.h r=pbone
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a7cc519c961
Move all scheduling related data structures to a new gc/Scheduling.h r=pbone

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a061e3873df0
Fix bustage due to missing explicit keyword r=me on a CLOSED TREE

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/40bbf952db46
https://hg.mozilla.org/mozilla-central/rev/5a7cc519c961
https://hg.mozilla.org/mozilla-central/rev/a061e3873df0