Closed Bug 1407682 Opened 2 years ago Closed 2 years ago

Refactor GC headers to reduce the number of inclusions

Categories

(Core :: JavaScript: GC, enhancement, P4)

55 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: jonco, Assigned: allstars.chh)

Details

Attachments

(12 files, 15 obsolete files)

2.67 KB, patch
jonco
: review+
jonco
: feedback+
Details | Diff | Splinter Review
15.42 KB, patch
jonco
: review+
Details | Diff | Splinter Review
8.34 KB, patch
allstars.chh
: review+
Details | Diff | Splinter Review
6.42 KB, patch
allstars.chh
: review+
Details | Diff | Splinter Review
27.59 KB, patch
allstars.chh
: review+
Details | Diff | Splinter Review
31.08 KB, patch
jonco
: review+
Details | Diff | Splinter Review
8.17 KB, patch
allstars.chh
: review+
Details | Diff | Splinter Review
2.40 KB, patch
jonco
: review+
Details | Diff | Splinter Review
11.60 KB, patch
jonco
: review+
Details | Diff | Splinter Review
7.99 KB, patch
allstars.chh
: review+
Details | Diff | Splinter Review
43.03 KB, patch
allstars.chh
: review+
Details | Diff | Splinter Review
16.70 KB, patch
allstars.chh
: review+
Details | Diff | Splinter Review
At the moment most GC header files end up getting included in over 400 cpp files.  I'm pretty sure this is unnecessary and leads to slower builds generally and long compile times after touching anything in these headers.

This bug is about improving this situation.

I'd also like to restructure things where possible so that each header has a clear role both in terms of functionality (what it does) and exposure (who includes it).
I made this P4 because it's not something that's either performance or safety related.  But I also don't want to see it delayed.  This investment would make a lot of our work easier.
Priority: -- → P4
(In reply to Jon Coppeard (:jonco) from comment #0)
> At the moment most GC header files end up getting included in over 400 cpp
> files.
Hi Jonco, could you point out which headers you're referring to?
So far I only found RootingAPI.h is included in many files, other headers in js/public and js/src/gc seem okay, some takes 50+ occurrences, but is still quite far from 400+.

Thanks
(In reply to Yoshi Huang [:allstars.chh] from comment #2)
Hi Yoshi,

I'm talking about the files under js/src/gc, and I'm not just talking about direct inclusion but indirect inclusion through a chain of includes.  So for example, gc/Heap.h is included in gc/Allocator.h which is included in jsobjinlines.h which is included a hundred or so places.  And it's also included in gc/AtomMarking.h which is included by gc/GCRuntime.h which gets included by a bunch of other stuff, and so it goes on.

I checked my numbers though (I'm using a python script I hacked together) and it's more like ~350 includes for most of the GC header files out of 375 .cpp source files (not counting test code).
Assignee: nobody → allstars.chh
As I see it we have four 'customers' for GC header files, which are roughly:

 1. system code: for example, initialisation and shutdown of the GC, setting in GC parameters, triggering collections
 2. users of GC things: the most common case, requires the rooting API and barriered pointers, GC vectors, etc
 3. providers of GC things: e.g. JSScript, js::Shape, they require Cell base classes, allocation, tracing and sweeping functions
 4. GC internals: our implementation

We should ensure that each header file has only one customer.
The following headers have multiple customers and should be split up:

 - gc/Marking.h: contains provider APIs (e.g. IsAboutToBeFinalized) and internal classes (e.g. GCMarker)
    => split out GCMarker and associated classes into a new GCMarker.h
 - gc/Heap.h, Heap-inl.h: contains provider classes (e.g. Cell base classes) and internal classes (e.g. Arena)
    => if possible, split out cell base classes into new Cell.h
 - gc/StoreBuffer.h, StoreBuffer-inl.h: this should be internal, but there are a couple of things we need to inline
    => try and move the parts necessary for inlining somewhere else
 - gc/Nursery.h, -inl.h: the same as for StoreBuffer.h
 - jsgc.h: the worst offender, this has a mix of everything
    => this file should be the system API only, and everything else should move out
(In reply to Yoshi Huang [:allstars.chh] from comment #2)
Yoshi, I was planning on working on this myself, but if you want to I'm happy to review patches.
(In reply to Jon Coppeard (:jonco) from comment #6)
> (In reply to Yoshi Huang [:allstars.chh] from comment #2)
> Yoshi, I was planning on working on this myself, but if you want to I'm
> happy to review patches.

Great, and also thanks for your valuable suggestions in comment 4 and comment 5.
Status: NEW → ASSIGNED
CC'ing other GC people so they are aware of this.
>  - jsgc.h: the worst offender, this has a mix of everything
>     => this file should be the system API only, and everything else should
> move out

I am still working on this part, will attach another patch for this.
Attachment #8921000 - Flags: feedback?(jcoppeard)
Attachment #8921001 - Flags: feedback?(jcoppeard)
Attachment #8921002 - Flags: feedback?(jcoppeard)
Attachment #8921003 - Flags: feedback?(jcoppeard)
Attachment #8921004 - Flags: feedback?(jcoppeard)
Attachment #8921005 - Flags: feedback?(jcoppeard)
Comment on attachment 8921000 [details] [diff] [review]
Part 1: move ArenaCellIndexBytes and MaxArenaCellIndex to HeapAPI.h

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

These constants are internal to the engine, so I don't think we want to move them to a public header file.
Attachment #8921000 - Flags: feedback?(jcoppeard)
Comment on attachment 8921001 [details] [diff] [review]
Part 2: Move NurseryChunk to Nursery.cpp

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

::: js/src/gc/Nursery.cpp
@@ +1105,5 @@
> +    MOZ_ASSERT(numChunks() > 0);
> +    return chunks_.back()->end();
> +}
> +
> +uintptr_t

Please make this MOZ_ALWAYS_INLINE.

::: js/src/gc/Nursery.h
@@ -14,5 @@
>  #include "jsalloc.h"
>  #include "jspubtd.h"
>  
>  #include "ds/BitArray.h"
> -#include "gc/Heap.h"

Great to see this go!

@@ -161,5 @@
>       * Check whether an arbitrary pointer is within the nursery. This is
>       * slower than IsInsideNursery(Cell*), but works on all types of pointers.
>       */
>      MOZ_ALWAYS_INLINE bool isInside(gc::Cell* cellp) const = delete;
> -    MOZ_ALWAYS_INLINE bool isInside(const void* p) const {

This method needs to be inline for performance reasons; we can't move this to Nursery.cpp.

Having said that, |chunk->start()| will always return a pointer to the same address as |chunk| itself, so you can make it compare against this pointer and keep the method inline without having the definition of NurseryChunk in this file.

@@ -440,5 @@
>      void updateNumChunks(unsigned newCount);
>      void updateNumChunksLocked(unsigned newCount,
>                                 AutoLockGCBgAlloc& lock);
>  
> -    MOZ_ALWAYS_INLINE uintptr_t allocationEnd() const {

It seems this isn't used anywhere and you can remove it.

@@ +430,2 @@
>  
> +    uintptr_t currentEnd() const;

Please keep this as MOZ_ALWAYS_INLINE even with the definition in Nursery.cpp.
Attachment #8921001 - Flags: feedback?(jcoppeard) → feedback+
Comment on attachment 8921000 [details] [diff] [review]
Part 1: move ArenaCellIndexBytes and MaxArenaCellIndex to HeapAPI.h

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

::: js/src/gc/Heap.h
@@ -362,5 @@
> - * cell is found by dividing by the cell alignment so not all indicies refer to
> - * valid cells.
> - */
> -const size_t ArenaCellIndexBytes = CellAlignBytes;
> -const size_t MaxArenaCellIndex = ArenaSize / CellAlignBytes;

Oh right, you needed this change for the next patch to be able to remove the include of Heap.h.

I think the problem with Heap.h is not these constants but the fact that it includes jsfriendapi.h.  Maybe it's possible to remove this somehow.
Comment on attachment 8921002 [details] [diff] [review]
Part 3: Move inline code from Nursery.h to Nursery-inl.h and Marking.cpp

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

::: js/src/gc/Marking.cpp
@@ +2892,5 @@
>      for (; vp != end; ++vp)
>          traverse(vp);
>  }
>  
> +void

Please add the |inline| keyword to match the original setup.

::: js/src/gc/Nursery.h
@@ -24,2 @@
>  #include "js/Vector.h"
> -#include "vm/SharedMem.h"

Nice!
Attachment #8921002 - Flags: feedback?(jcoppeard) → feedback+
Comment on attachment 8921003 [details] [diff] [review]
Part 4: Seperate GCMarker from Marking.cpp

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

I should have been more explicit in the description, but this bug is just about changing the header files.  I don't think we want to split the .cpp files (at the moment anyway, that might come later).

In this case there's a lot of inlining that happens in the marking code and I think we need to keep all that code together in one file in Marking.cpp.  I don't think we should split the code into GCMarker.cpp, only the header file.

Secondly, we should move the related classes that GCMarker uses into GCMarker.h too, i.e. MarkStack and MarkStackIter (and anything else it uses).  I think this ends up with Marking.h having only a few functions in it, and this is fine.
Attachment #8921003 - Flags: feedback?(jcoppeard)
Comment on attachment 8921000 [details] [diff] [review]
Part 1: move ArenaCellIndexBytes and MaxArenaCellIndex to HeapAPI.h

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

Having thought about this some more, this is fine.  We already have a bunch of internal constants here, and it makes sense to keep stuff like this together.
Attachment #8921000 - Flags: feedback+
Attachment #8921000 - Flags: review?(jcoppeard)
Addressed comment 17.
Attachment #8921001 - Attachment is obsolete: true
Attachment #8921412 - Flags: review?(jcoppeard)
Comment on attachment 8921004 [details] [diff] [review]
Part 5: Move inline code from StoreBuffer.h to StoreBuffer.cpp and StoreBuffer-inl.h

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

change to r?
Attachment #8921004 - Flags: feedback?(jcoppeard) → review?(jcoppeard)
rebase on latest patches.
Attachment #8921005 - Attachment is obsolete: true
Attachment #8921005 - Flags: feedback?(jcoppeard)
Attachment #8921415 - Flags: review?(jcoppeard)
Attachment #8921421 - Flags: review?(jcoppeard)
Attachment #8921414 - Flags: review?(jcoppeard)
The build fails on try 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c938fb93a1563b8c1c62e272258886867bc82bd4&selectedJob=139159921

However it works in my local machine, I'll check why it fails on try.
Attachment #8921000 - Flags: review?(jcoppeard) → review+
Comment on attachment 8921412 [details] [diff] [review]
Part 2: Move NurseryChunk to Nursery.cpp. v2

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

::: js/src/gc/Nursery.h
@@ +166,5 @@
>       */
>      MOZ_ALWAYS_INLINE bool isInside(gc::Cell* cellp) const = delete;
>      MOZ_ALWAYS_INLINE bool isInside(const void* p) const {
>          for (auto chunk : chunks_) {
> +            if (uintptr_t(p) - uintptr_t(&chunk) < gc::ChunkSize)

This needs to be |chunk| not |&chunk| here since it's already a NurseryChunk*.
Attachment #8921412 - Flags: review?(jcoppeard) → review+
Comment on attachment 8921413 [details] [diff] [review]
Part 3: Move inline code from Nursery.h to Nursery-inl.h and Marking.cpp. v2

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

::: js/src/gc/Nursery.h
@@ +167,5 @@
>          return false;
>      }
>  
>      template<typename T>
> +    bool isInside(const SharedMem<T>& p) const;

Please mark this |inline| here so that if something forgets to include Nursery-inl.h they will get an error (or warning?) at compile time rather than an error at link time.
Attachment #8921413 - Flags: review?(jcoppeard) → review+
Comment on attachment 8921414 [details] [diff] [review]
Part 4: Separate GCMarker class from Marking.h. v2

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

Looks good.

::: js/src/gc/Marking.h
@@ -17,5 @@
>  #include "gc/Tracer.h"
>  #include "js/GCAPI.h"
>  #include "js/HeapAPI.h"
>  #include "js/SliceBudget.h"
>  #include "js/TracingAPI.h"

I think you can get rid of a few includes here too, and some forward declarations below.

@@ +36,5 @@
>  class JitCode;
>  } // namespace jit
>  
>  static const size_t NON_INCREMENTAL_MARK_STACK_BASE_CAPACITY = 4096;
>  static const size_t INCREMENTAL_MARK_STACK_BASE_CAPACITY = 32768;

These constants should go in GCMarker.h.

Also all the ThingIsPermanentAtomOrWellKnownSymbol definitions at the bottom belong there too, as they are internal to the GC.
Attachment #8921414 - Flags: review?(jcoppeard) → review+
Comment on attachment 8921004 [details] [diff] [review]
Part 5: Move inline code from StoreBuffer.h to StoreBuffer.cpp and StoreBuffer-inl.h

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

::: js/src/gc/StoreBuffer.h
@@ +165,2 @@
>  
> +        bool isAboutToOverflow() const;

A lot of this code is performance sensitive and must continue to be inline.  Everything in this header except constructors/destructors and sizeofExcludingThis methods must move to Nursery-inl.h rather than Nursery.cpp.
Attachment #8921004 - Flags: review?(jcoppeard)
Comment on attachment 8921415 [details] [diff] [review]
Part 6: Add Cell.cpp, seperate Cell-related class from Heap.h v2

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

Thanks for doing this, but like the store buffer header all these methods need to remain inline.  I'd suggest just doing everything inline in Cell.h (using Cell-inl.h probably requires that to be included in too many places).
Attachment #8921415 - Flags: review?(jcoppeard)
Comment on attachment 8921416 [details] [diff] [review]
Part 7: Move GCParallelTask to its own header

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

I'm not sure we want to split out all these classes into new header files, but this one is a good candidate.

I had to #include "vm/SharedMem.h" in wasm/WasmInstance.h to get this to compile BTW.
Attachment #8921416 - Flags: review?(jcoppeard) → review+
Comment on attachment 8921417 [details] [diff] [review]
Part 8 : Move ZoneList to its own header.

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

This is pretty small to warrant its own file.  I think we should just move this into GCRuntime.h.
Attachment #8921417 - Flags: review?(jcoppeard)
Comment on attachment 8921418 [details] [diff] [review]
Part 9: Move Auto* classes to its own header.

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

::: js/src/jsgc.h
@@ -1143,5 @@
> - * that they are live. Use of this class is highly discouraged. Please carefully
> - * read the comment in vm/Runtime.h above |suppressGC| and take all appropriate
> - * precautions before instantiating this class.
> - */
> -class MOZ_RAII JS_HAZ_GC_SUPPRESSED AutoSuppressGC

This one is really a system API and is used in a bunch of places throughout the engine.  This should stay in jsgc.h.

@@ -1160,4 @@
>  JSObject*
>  NewMemoryStatisticsObject(JSContext* cx);
>  
> -struct MOZ_RAII AutoAssertNoNurseryAlloc

This can move to GCInternals.h.

@@ -1193,5 @@
> - * emplaced during the constructor, but token-requiring functions want to
> - * require a reference to a base class instance. That said, you can always pass
> - * in the Maybe<> field as the token.
> - */
> -class MOZ_RAII AutoAssertHeapBusy {

It seems AutoAssertHeapBusy is unused.  Can you remove it?

@@ -1219,5 @@
> -/*
> - * A class that serves as a token that the nursery in the current thread's zone
> - * group is empty.
> - */
> -class MOZ_RAII AutoAssertEmptyNursery

Please move this one and AutoEmptyNursery to GCInternals.h.

@@ -1271,5 @@
>  
>  } /* namespace gc */
>  
> -/* Use this to avoid assertions when manipulating the wrapper map. */
> -class MOZ_RAII AutoDisableProxyCheck

This is really a system API too, so this should stay in jsgc.h.

@@ -1282,5 @@
> -    AutoDisableProxyCheck() {}
> -#endif
> -};
> -
> -struct MOZ_RAII AutoDisableCompactingGC

The same goes for this one.
Attachment #8921418 - Flags: review?(jcoppeard)
Comment on attachment 8921419 [details] [diff] [review]
Part 10: Move GCHelperState to its own header

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

This looks good.
Attachment #8921419 - Flags: review?(jcoppeard) → review+
Comment on attachment 8921420 [details] [diff] [review]
Part 11: Move ArenaList to its own header

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

Nice.

::: js/src/gc/Allocator.cpp
@@ +297,5 @@
>      // inside a GC.
>      Zone *zone = cx->zone();
>      MOZ_ASSERT(!JS::CurrentThreadIsHeapBusy(), "allocating while under GC");
>  
> +    return cx->arenas()->allocateFromArena(zone, thingKind, ShouldCheckThresholds::CheckThresholds);

Why do we need to qualify this now?
Attachment #8921420 - Flags: review?(jcoppeard) → review+
Comment on attachment 8921421 [details] [diff] [review]
Part 12: don't include jsgc.h

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

This is great.
Attachment #8921421 - Flags: review?(jcoppeard) → review+
Comment on attachment 8921422 [details] [diff] [review]
Part 13: remove unneccesary includes

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

::: js/src/vm/Xdr.h
@@ +202,5 @@
>      }
>      virtual LifoAlloc& lifoAlloc() const;
>  
>      virtual bool hasOptions() const { return false; }
> +    virtual const JS::ReadOnlyCompileOptions& options() {

Why do we need to qualify these now?
(In reply to Jon Coppeard (:jonco) from comment #38)
> Comment on attachment 8921004 [details] [diff] [review]
> Part 5: Move inline code from StoreBuffer.h to StoreBuffer.cpp and
> StoreBuffer-inl.h
> 
> Review of attachment 8921004 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/gc/StoreBuffer.h
> @@ +165,2 @@
> >  
> > +        bool isAboutToOverflow() const;
> 
> A lot of this code is performance sensitive and must continue to be inline. 
> Everything in this header except constructors/destructors and
> sizeofExcludingThis methods must move to Nursery-inl.h rather than
> Nursery.cpp.

Hi Jonco, just to be sure, you mean 
"Everything in this header except ... must move to "StoreBuffer-inl.h" rather than "StoreBuffer.cpp".
right?
(In reply to Jon Coppeard (:jonco) from comment #40)
> I had to #include "vm/SharedMem.h" in wasm/WasmInstance.h to get this to
> compile BTW.
I did this in my Part 6 patch. (Your original comment 40 is for Part 7 patch).
Comment on attachment 8921420 [details] [diff] [review]
Part 11: Move ArenaList to its own header

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

::: js/src/gc/Allocator.cpp
@@ +297,5 @@
>      // inside a GC.
>      Zone *zone = cx->zone();
>      MOZ_ASSERT(!JS::CurrentThreadIsHeapBusy(), "allocating while under GC");
>  
> +    return cx->arenas()->allocateFromArena(zone, thingKind, ShouldCheckThresholds::CheckThresholds);

In ArenaList I change ShouldCheckThresholds to an enum class, and this is for forward declaration in GCRuntime.h.
(In reply to Jon Coppeard (:jonco) from comment #46)
> Comment on attachment 8921422 [details] [diff] [review]
> Part 13: remove unneccesary includes
> 
> Review of attachment 8921422 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/vm/Xdr.h
> @@ +202,5 @@
> >      }
> >      virtual LifoAlloc& lifoAlloc() const;
> >  
> >      virtual bool hasOptions() const { return false; }
> > +    virtual const JS::ReadOnlyCompileOptions& options() {
> 
> Why do we need to qualify these now?

I met a compilation error before, so I added JS:: to it, but now I tried my latest codebase it won't have error now, I could revert the JS:: change in Xdr.h
(In reply to Yoshi Huang [:allstars.chh] (OOO 10.26 ~ 11.2) from comment #47)
> Hi Jonco, just to be sure, you mean 
> "Everything in this header except ... must move to "StoreBuffer-inl.h"
> rather than "StoreBuffer.cpp".
> right?

Sorry, you are right.  I meant StoreBuffer-inl.h.
(In reply to Yoshi Huang [:allstars.chh] (OOO 10.26 ~ 11.2) from comment #49)
> In ArenaList I change ShouldCheckThresholds to an enum class, and this is
> for forward declaration in GCRuntime.h.

Oh, OK that's fine then.

> I met a compilation error before, so I added JS:: to it, but now I tried my latest codebase it won't have error now, I could revert the JS:: change in Xdr.h

OK.  It would be better not to change this if it's not necessary.
addressed comment 39, also obsolete original part 5.
Attachment #8921004 - Attachment is obsolete: true
Attachment #8921415 - Attachment is obsolete: true
Attachment #8921825 - Flags: review?(jcoppeard)
addressed comment 41.
Attachment #8921417 - Attachment is obsolete: true
Attachment #8921827 - Flags: review?(jcoppeard)
Comment on attachment 8921825 [details] [diff] [review]
Part 5: Seperate Cell-related class from Heap.h. v3

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

::: js/src/gc/Cell.cpp
@@ +17,5 @@
> +
> +#endif
> +} /* namespace gc */
> +
> +} /* namespace js */

Please remove this emtpy file.

::: js/src/gc/Heap.cpp
@@ +13,5 @@
> +
> +namespace gc {
> +
> +void
> +AssertValidColorBit(const TenuredCell* thing, ColorBit colorBit)

It's not worth creating a new file just for this function.  This should probably continue to live in the header file.
Attachment #8921825 - Flags: review?(jcoppeard) → review+
Attachment #8921827 - Flags: review?(jcoppeard) → review+
Attachment #8921828 - Flags: review?(jcoppeard) → review+
Comment on attachment 8921422 [details] [diff] [review]
Part 13: remove unneccesary includes

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

::: js/src/vm/Xdr.h
@@ +202,5 @@
>      }
>      virtual LifoAlloc& lifoAlloc() const;
>  
>      virtual bool hasOptions() const { return false; }
> +    virtual const JS::ReadOnlyCompileOptions& options() {

Please do revert the namespace qualifier here if possible.
Attachment #8921422 - Flags: review?(jcoppeard) → review+
(In reply to Jon Coppeard (:jonco) from comment #63)
> > +void
> > +AssertValidColorBit(const TenuredCell* thing, ColorBit colorBit)
> 
> It's not worth creating a new file just for this function.  This should
> probably continue to live in the header file.

Oh right, this needs the definition of Arena which we no longer have.  But this assert is more complicated than it needs to be since we already enforce a minimum number of mark bits with MarkBitsPerCell.  We can just compare against that.
This is going to bitrot pretty quickly, so I'm going to make the code review changes myself and land the patches.
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/70ef92e6dfb4
Part 1: move ArenaCellIndexBytes and MaxArenaCellIndex r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/483e663b4d25
Part 2: Move NurseryChunk to Nursery.cpp. r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e6a6411d0ae
Part 3: Move inline code from Nursery.h to Nursery-inl.h and Marking.cpp. r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/c27975d96f23
Part 4: Separate GCMarker class from Marking.h. r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe3491c986ff
Part 5: Seperate Cell-related class from Heap.h to Cell.h r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e2b188624ee
Part 6: Move GCParallelTask to its own header. r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2cf435b4113
Part 7 : Move ZoneList from jsgc.h to GCRuntime r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef8e3aa42031
Part 8: Move Auto* classes to GCInternals.h. r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/be919225af75
Part 9: Move GCHelperState to its own header. r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/5621a2cc7fb9
Part 10: Move ArenaList to its own header. r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/6410d5638a98
Part 11: Remove unnecessary inclusion of jsgc.h. r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/5606cb25cc92
Part 13: Remove unneccesary includes r=jonco
Thanks for the review and the push, Jonco, it helps me a lot and saves me a lot of time to rebase it. :)
(In reply to Yoshi Huang [:allstars.chh] from comment #69)
No problem, thanks for working on this.
You need to log in before you can comment on or make changes to this bug.