Skip to content

Commit

Permalink
[Local GC] Move workstation GC DAC globals to a struct shared between…
Browse files Browse the repository at this point in the history
… GC and DAC (dotnet#9255)

* [Local GC] Move workstation GC DAC globals to a struct shared between
the GC and the DAC

* (Some) code review feedback and bug fixes for issues found while debugging on OSX

* Address some code review feedback:
 1. Make g_gcDacGlobals a pointer and dacvar on the VM side, so
    that publishing the GC dac vars is done atomically (through
    a pointer assignment). This fixes a race that Noah noticed.
 2. Remove the requirement for the GC's generation class struct
    to be known at compile-time, by using a dacvar as the size
    of the generation class at run-time (for pointer arithmetic)
 3. Move all DAC-interesting fields to be at the start of GC
    internal classes, so that the DAC does not need to know the
    size or exact layout of the class past the fields it cares
    about.

* Split the definition of the size of several arrays across the SOS/DAC and GC/DAC interfaces, and add static asserts that they are the same

* Repair the Windows Release build

* Implement the GC DAC scheme for Server GC and eliminate the duplicate GC dac vars

* Some work

* Decouple use of the GC generation table from a write barrier by having the EE store a copy of the global during initialization

* Actually make it work with server GC

* Checkpoint

* Checkpoint where everything works

* Code cleanup

* Fix debugger test failures

* Additional code cleanup

* Address code review feedback by adding a static assert and standardizing the way that we iterate over the generation table

* Repair the Windows x86 build

* Revert "Decouple use of the GC generation table from a write barrier by having the EE store a copy of the global during initialization"

This reverts commit 573f61a.

* Revert "Repair the Windows x86 build"

This reverts commit 188c22d.

* Partial revert, move `generation_table` back the global namespace for a single-proc allocation helper

* Fix a debugger test failure

* Repair crash dump scenarios
  • Loading branch information
swgillespie authored and jorive committed May 4, 2017
1 parent f40d367 commit 9678e47
Show file tree
Hide file tree
Showing 25 changed files with 764 additions and 468 deletions.
10 changes: 5 additions & 5 deletions src/ToolBox/SOS/Strike/strike.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8838,28 +8838,28 @@ void PrintInterestingGCInfo(DacpGCInterestingInfoData* dataPerHeap)
{
ExtOut("Interesting data points\n");
size_t* data = dataPerHeap->interestingDataPoints;
for (int i = 0; i < NUM_GC_DATA_POINTS; i++)
for (int i = 0; i < DAC_NUM_GC_DATA_POINTS; i++)
{
ExtOut("%20s: %d\n", str_interesting_data_points[i], data[i]);
}

ExtOut("\nCompacting reasons\n");
data = dataPerHeap->compactReasons;
for (int i = 0; i < MAX_COMPACT_REASONS_COUNT; i++)
for (int i = 0; i < DAC_MAX_COMPACT_REASONS_COUNT; i++)
{
ExtOut("[%s]%35s: %d\n", (gc_heap_compact_reason_mandatory_p[i] ? "M" : "W"), str_heap_compact_reasons[i], data[i]);
}

ExtOut("\nExpansion mechanisms\n");
data = dataPerHeap->expandMechanisms;
for (int i = 0; i < MAX_EXPAND_MECHANISMS_COUNT; i++)
for (int i = 0; i < DAC_MAX_EXPAND_MECHANISMS_COUNT; i++)
{
ExtOut("%30s: %d\n", str_heap_expand_mechanisms[i], data[i]);
}

ExtOut("\nOther mechanisms enabled\n");
data = dataPerHeap->bitMechanisms;
for (int i = 0; i < MAX_GC_MECHANISM_BITS_COUNT; i++)
for (int i = 0; i < DAC_MAX_GC_MECHANISM_BITS_COUNT; i++)
{
ExtOut("%20s: %d\n", str_bit_mechanisms[i], data[i]);
}
Expand All @@ -8881,7 +8881,7 @@ DECLARE_API(DumpGCData)

DacpGCInterestingInfoData interestingInfo;
interestingInfo.RequestGlobal(g_sos);
for (int i = 0; i < MAX_GLOBAL_GC_MECHANISMS_COUNT; i++)
for (int i = 0; i < DAC_MAX_GLOBAL_GC_MECHANISMS_COUNT; i++)
{
ExtOut("%-30s: %d\n", str_gc_global_mechanisms[i], interestingInfo.globalMechanisms[i]);
}
Expand Down
1 change: 0 additions & 1 deletion src/debug/daccess/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ include_directories(BEFORE ${VM_DIR})
include_directories(BEFORE ${VM_DIR}/${ARCH_SOURCES_DIR})
include_directories(BEFORE ${CMAKE_CURRENT_SOURCE_DIR})
include_directories(${CLR_DIR}/src/debug/ee)
include_directories(${CLR_DIR}/src/gc)
include_directories(${CLR_DIR}/src/gcdump)

if(CLR_CMAKE_PLATFORM_UNIX)
Expand Down
7 changes: 4 additions & 3 deletions src/debug/daccess/daccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7934,7 +7934,8 @@ STDAPI OutOfProcessExceptionEventDebuggerLaunchCallback(__in PDWORD pContext,

// DacHandleEnum

#include "handletablepriv.h"
// TODO(Local GC) - The DAC should not include GC headers
#include "../../gc/handletablepriv.h"
#include "comcallablewrapper.h"

DacHandleWalker::DacHandleWalker()
Expand Down Expand Up @@ -7976,7 +7977,7 @@ HRESULT DacHandleWalker::Init(ClrDataAccess *dac, UINT types[], UINT typeCount,
{
SUPPORTS_DAC;

if (gen < 0 || gen > (int)GCHeapUtilities::GetMaxGeneration())
if (gen < 0 || gen > (int)*g_gcDacGlobals->max_gen)
return E_INVALIDARG;

mGenerationFilter = gen;
Expand Down Expand Up @@ -8091,7 +8092,7 @@ bool DacHandleWalker::FetchMoreHandles(HANDLESCANPROC callback)
HndScanHandlesForGC(hTable, callback,
(LPARAM)&param, 0,
&handleType, 1,
mGenerationFilter, GCHeapUtilities::GetMaxGeneration(), 0);
mGenerationFilter, *g_gcDacGlobals->max_gen, 0);
else
HndEnumHandles(hTable, &handleType, 1, callback, (LPARAM)&param, 0, FALSE);
}
Expand Down
31 changes: 18 additions & 13 deletions src/debug/daccess/dacdbiimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include "comcallablewrapper.h"
#endif // FEATURE_COMINTEROP

#include "request_common.h"

//-----------------------------------------------------------------------------
// Have standard enter and leave macros at the DacDbi boundary to enforce
// standard behavior.
Expand Down Expand Up @@ -6578,7 +6580,6 @@ HRESULT DacHeapWalker::ListNearObjects(CORDB_ADDRESS obj, CORDB_ADDRESS *pPrev,
return hr;
}

#include "gceewks.cpp"
HRESULT DacHeapWalker::InitHeapDataWks(HeapData *&pHeaps, size_t &pCount)
{
// Scrape basic heap details
Expand All @@ -6587,31 +6588,35 @@ HRESULT DacHeapWalker::InitHeapDataWks(HeapData *&pHeaps, size_t &pCount)
if (pHeaps == NULL)
return E_OUTOFMEMORY;

pHeaps[0].YoungestGenPtr = (CORDB_ADDRESS)WKS::generation_table[0].allocation_context.alloc_ptr;
pHeaps[0].YoungestGenLimit = (CORDB_ADDRESS)WKS::generation_table[0].allocation_context.alloc_limit;
dac_generation gen0 = *GenerationTableIndex(g_gcDacGlobals->generation_table, 0);
dac_generation gen1 = *GenerationTableIndex(g_gcDacGlobals->generation_table, 1);
dac_generation gen2 = *GenerationTableIndex(g_gcDacGlobals->generation_table, 2);
dac_generation loh = *GenerationTableIndex(g_gcDacGlobals->generation_table, 3);
pHeaps[0].YoungestGenPtr = (CORDB_ADDRESS)gen0.allocation_context.alloc_ptr;
pHeaps[0].YoungestGenLimit = (CORDB_ADDRESS)gen0.allocation_context.alloc_limit;

pHeaps[0].Gen0Start = (CORDB_ADDRESS)WKS::generation_table[0].allocation_start;
pHeaps[0].Gen0End = (CORDB_ADDRESS)WKS::gc_heap::alloc_allocated.GetAddr();
pHeaps[0].Gen1Start = (CORDB_ADDRESS)WKS::generation_table[1].allocation_start;
pHeaps[0].Gen0Start = (CORDB_ADDRESS)gen0.allocation_start;
pHeaps[0].Gen0End = (CORDB_ADDRESS)*g_gcDacGlobals->alloc_allocated;
pHeaps[0].Gen1Start = (CORDB_ADDRESS)gen1.allocation_start;

// Segments
int count = GetSegmentCount(WKS::generation_table[NUMBERGENERATIONS-1].start_segment);
count += GetSegmentCount(WKS::generation_table[NUMBERGENERATIONS-2].start_segment);
int count = GetSegmentCount(loh.start_segment);
count += GetSegmentCount(gen2.start_segment);

pHeaps[0].SegmentCount = count;
pHeaps[0].Segments = new (nothrow) SegmentData[count];
if (pHeaps[0].Segments == NULL)
return E_OUTOFMEMORY;

// Small object heap segments
WKS::heap_segment *seg = WKS::generation_table[NUMBERGENERATIONS-2].start_segment;
DPTR(dac_heap_segment) seg = gen2.start_segment;
int i = 0;
for (; seg && (i < count); ++i)
{
pHeaps[0].Segments[i].Start = (CORDB_ADDRESS)seg->mem;
if (seg == WKS::gc_heap::ephemeral_heap_segment)
if (seg.GetAddr() == (TADDR)*g_gcDacGlobals->ephemeral_heap_segment)
{
pHeaps[0].Segments[i].End = (CORDB_ADDRESS)WKS::gc_heap::alloc_allocated.GetAddr();
pHeaps[0].Segments[i].End = (CORDB_ADDRESS)*g_gcDacGlobals->alloc_allocated;
pHeaps[0].Segments[i].Generation = 1;
pHeaps[0].EphemeralSegment = i;
}
Expand All @@ -6625,7 +6630,7 @@ HRESULT DacHeapWalker::InitHeapDataWks(HeapData *&pHeaps, size_t &pCount)
}

// Large object heap segments
seg = WKS::generation_table[NUMBERGENERATIONS-1].start_segment;
seg = loh.start_segment;
for (; seg && (i < count); ++i)
{
pHeaps[0].Segments[i].Generation = 3;
Expand Down Expand Up @@ -7127,7 +7132,7 @@ void DacDbiInterfaceImpl::GetGCHeapInformation(COR_HEAPINFO * pHeapInfo)
DD_ENTER_MAY_THROW;

size_t heapCount = 0;
pHeapInfo->areGCStructuresValid = GCScan::GetGcRuntimeStructuresValid();
pHeapInfo->areGCStructuresValid = *g_gcDacGlobals->gc_structures_invalid_cnt == 0;

#ifdef FEATURE_SVR_GC
if (GCHeapUtilities::IsServerHeap())
Expand Down
4 changes: 4 additions & 0 deletions src/debug/daccess/dacfn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
#include <virtualcallstub.h>
#include "peimagelayout.inl"

#include "gcinterface.h"
#include "gcinterface.dac.h"


DacTableInfo g_dacTableInfo;
DacGlobals g_dacGlobals;

Expand Down
6 changes: 6 additions & 0 deletions src/debug/daccess/enummem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,12 @@ HRESULT ClrDataAccess::EnumMemCLRStatic(IN CLRDataEnumMemoryFlags flags)

ReportMem( m_globalBase + g_dacGlobals.dac__g_FCDynamicallyAssignedImplementations,
sizeof(TADDR)*ECall::NUM_DYNAMICALLY_ASSIGNED_FCALL_IMPLEMENTATIONS);

// We need all of the dac variables referenced by the GC DAC global struct.
// This struct contains pointers to pointers, so we first dereference the pointers
// to obtain the location of the variable that's reported.
#define GC_DAC_VAR(type, name) ReportMem(g_gcDacGlobals->name.GetAddr(), sizeof(type));
#include "../../gc/gcinterface.dacvars.def"
}
EX_CATCH_RETHROW_ONLY_COR_E_OPERATIONCANCELLED

Expand Down
5 changes: 5 additions & 0 deletions src/debug/daccess/gcinterface.dac.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#include "../../gc/gcinterface.dac.h"
Loading

0 comments on commit 9678e47

Please sign in to comment.