-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Description
As part of the LocalGC project I’d like to stop sharing the GC globals between the GC code and the WB (Write Barrier) code. The globals are (note I am not including things that don’t run on customers machines by default like debug only code or GCShadow stuff):
g_card_table
g_lowest_address
g_highest_address
g_ephemeral_low
g_ephemeral_high
The last 2 are only changed when EE is suspended so there’s no significance in the order they are set wrt anything else – managed threads are not running at this point anyway. But it would still be good to not share them.
The first 3 are what causes all this intricacy. There is one invariant that we need to stick to which is
The WB code must see the updated g_card_table before they see updated g_lowest/highest
So the WB side just needs to make sure they are updated in that order.
On the GC side, since GC only uses these either when EE is stopped or when it’s under a global lock. As long as we are not stopped in the middle of setting these 3 values we are fine (the rare race that occurs today is due to the fact that we can be stopped in the middle).
So instead of declaring these on the GC side, we should have 2 sets of copies for these, one on GC side; the other on CLR side -
Remove the global declaration of them from gccommon.cpp and add them on the CLR side.
Rename them on the GC side so it’s clear they are separate. Replace g_ to g_gc_.
Pass them in the StompWriteBarrierResize and StompWriteBarrierEphemeral which will be part of the to GCToEEInterface/IGCToCLR interface.
// Consider creating a struct that includes all the globals so we don't need to pass in so many params
void StompWriteBarrierResize(bool isRuntimeSuspended, bool bReqUpperBoundsCheck, void* new_card_table, void* new_lowest_address, void new_highest_address);
// I would just get rid of the isRuntimeSuspesnded parameter as it's never called with false.
void WriteBarrierManager::UpdateEphemeralBounds(void* new_ephemeral_low, void* new_ephemeral_high);
In GC grow_brick_card_tables is where we might need to change the first 3 values. Right now it has this sequence:
g_card_table = new_card_table;
StompWriteBarrierResize();
GCToOSInterface::FlushProcessWriteBuffers();
g_lowest_address = new_lowest_address;
VolatileStore(&g_highest_address, new_highest_address);
This will be changed to:
g_gc_card_table = new_card_table;
g_gc_lowest_address = new_lowest_address;
g_gc_highest_address = new_highest_address;
StompWriteBarrierResize(param0, param1, g_gc_card_table, g_gc_lowest_address, g_gc_highest_address);
And in StompWriteBarrierResize,
Do what StompWriteBarrierResize currently does (ie, might suspend EE to change WB type; bash the g_card_table value) and then
g_card_table = new_card_table;
FlushProcessWriteBuffers();
g_lowest_address = new_lowest_address;
VolatileStore(&g_highest_address, new_highest_address);
@swgillespie Note I didn't include the software WW stuff... feel free to add that as we discussed.