Closed Bug 1437602 Opened 2 years ago Closed 2 years ago

Refactor gc/Zone.h a little


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




Tracking Status
firefox60 --- fixed


(Reporter: jonco, Assigned: jonco)



(2 files, 1 obsolete file)

Attached patch zone-1-split-out-iterators (obsolete) — Splinter Review
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.
Attachment #8950284 - Flags: review?(pbone)
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.
Attachment #8950286 - Flags: review?(pbone)
Priority: -- → P3
Comment on attachment 8950284 [details] [diff] [review]

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.


::: 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.
Attachment #8950284 - Flags: review?(pbone) → review+
Comment on attachment 8950286 [details] [diff] [review]

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+.
Attachment #8950286 - Flags: review?(pbone) → review+
(In reply to Paul Bone [:pbone] from comment #2)
> Should this commented out line be deleted?

Ugh, yes, I forgot to take those out.

I had to rename GCIterators.h to GCIterators-inl.h because our style checker complained about it including jsgcincludes.h.  Carrying r+.
Attachment #8950284 - Attachment is obsolete: true
Attachment #8951254 - Flags: review+
Pushed by
Split out zone and compartment iterators from gc/Zone.h r=pbone
Move all scheduling related data structures to a new gc/Scheduling.h r=pbone
Pushed by
Fix bustage due to missing explicit keyword r=me on a CLOSED TREE
You need to log in before you can comment on or make changes to this bug.