Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instrumentation #100294

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Instrumentation #100294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
aa378a5
Instrumentation to test commit accounting
cshung Aug 9, 2024
399b218
Experiment: always run segments
cshung Aug 9, 2024
d0ec815
Use the right bucket for committing mark array
cshung Aug 9, 2024
afb2b98
Revert "Use the right bucket for committing mark array"
cshung Aug 10, 2024
db97a84
Ignore Refresh test case
cshung Aug 12, 2024
bd052be
Revert "Move a regression test out of CI (#80399)"
cshung Aug 14, 2024
8f8cc56
WIP: Commit reason tracking
cshung Aug 12, 2024
fab32ad
Failing the test
cshung Aug 16, 2024
12969bb
Revert "Failing the test"
cshung Aug 16, 2024
31ae7c2
Hacking test script
cshung Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 26 additions & 29 deletions src/coreclr/gc/gc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6066,6 +6066,7 @@ void gc_heap::release_segment (heap_segment* sg)
FIRE_EVENT(GCFreeSegment_V1, heap_segment_mem(sg));
size_t reserved_size = (uint8_t*)heap_segment_reserved (sg) - (uint8_t*)sg;
reduce_committed_bytes (
28973,
sg,
((uint8_t*)heap_segment_committed (sg) - (uint8_t*)sg),
(int) heap_segment_oh (sg)
Expand Down Expand Up @@ -7289,7 +7290,7 @@ bool gc_heap::virtual_alloc_commit_for_heap (void* addr, size_t size, int h_numb
return GCToOSInterface::VirtualCommit(addr, size);
}

bool gc_heap::virtual_commit (void* address, size_t size, int bucket, int h_number, bool* hard_limit_exceeded_p)
bool gc_heap::virtual_commit (int reason, void* address, size_t size, int bucket, int h_number, bool* hard_limit_exceeded_p)
{
/**
* Here are all the possible cases for the commits:
Expand All @@ -7309,7 +7310,7 @@ bool gc_heap::virtual_commit (void* address, size_t size, int bucket, int h_numb
assert(bucket != recorded_committed_free_bucket);
#endif //USE_REGIONS

dprintf(3, ("commit-accounting: commit in %d [%p, %p) for heap %d", bucket, address, ((uint8_t*)address + size), h_number));
STRESS_LOG_VA (1, ("commit-accounting: commit in %d [%p, %p) for heap %d with reason %d", bucket, address, ((uint8_t*)address + size), h_number, reason));
bool should_count =
#ifdef USE_REGIONS
true;
Expand Down Expand Up @@ -7402,12 +7403,12 @@ bool gc_heap::virtual_commit (void* address, size_t size, int bucket, int h_numb
return commit_succeeded_p;
}

void gc_heap::reduce_committed_bytes (void* address, size_t size, int bucket, int h_number, bool decommit_succeeded_p)
void gc_heap::reduce_committed_bytes (int reason, void* address, size_t size, int bucket, int h_number, bool decommit_succeeded_p)
{
assert(0 <= bucket && bucket < recorded_committed_bucket_counts);
assert(bucket < total_oh_count || h_number == -1);

dprintf(3, ("commit-accounting: decommit in %d [%p, %p) for heap %d", bucket, address, ((uint8_t*)address + size), h_number));
STRESS_LOG_VA (1, ("commit-accounting: decommit in %d [%p, %p) for heap %d with reason", bucket, address, ((uint8_t*)address + size), h_number, reason));

#ifndef USE_REGIONS
if (bucket != recorded_committed_ignored_bucket)
Expand Down Expand Up @@ -7435,7 +7436,7 @@ void gc_heap::reduce_committed_bytes (void* address, size_t size, int bucket, in
}
}

bool gc_heap::virtual_decommit (void* address, size_t size, int bucket, int h_number)
bool gc_heap::virtual_decommit (int reason, void* address, size_t size, int bucket, int h_number)
{
/**
* Here are all possible cases for the decommits:
Expand All @@ -7450,7 +7451,7 @@ bool gc_heap::virtual_decommit (void* address, size_t size, int bucket, int h_nu

bool decommit_succeeded_p = ((bucket != recorded_committed_bookkeeping_bucket) && use_large_pages_p) ? true : GCToOSInterface::VirtualDecommit (address, size);

reduce_committed_bytes (address, size, bucket, h_number, decommit_succeeded_p);
reduce_committed_bytes (reason, address, size, bucket, h_number, decommit_succeeded_p);

return decommit_succeeded_p;
}
Expand Down Expand Up @@ -9086,7 +9087,7 @@ void gc_heap::destroy_card_table_helper (uint32_t* c_table)
uint8_t* highest = card_table_highest_address (c_table);
get_card_table_element_layout(lowest, highest, card_table_element_layout);
size_t result = card_table_element_layout[seg_mapping_table_element + 1];
gc_heap::reduce_committed_bytes (&card_table_refcount(c_table), result, recorded_committed_bookkeeping_bucket, -1, true);
gc_heap::reduce_committed_bytes (28972, &card_table_refcount(c_table), result, recorded_committed_bookkeeping_bucket, -1, true);

// If we don't put the mark array committed in the ignored bucket, then this is where to account for the decommit of it
}
Expand Down Expand Up @@ -9346,7 +9347,7 @@ bool gc_heap::inplace_commit_card_table (uint8_t* from, uint8_t* to)
bool succeed;
if (commit_sizes[i] > 0)
{
succeed = virtual_commit (commit_begins[i], commit_sizes[i], recorded_committed_bookkeeping_bucket);
succeed = virtual_commit (28967, commit_begins[i], commit_sizes[i], recorded_committed_bookkeeping_bucket);
if (!succeed)
{
failed_commit = i;
Expand All @@ -9368,7 +9369,7 @@ bool gc_heap::inplace_commit_card_table (uint8_t* from, uint8_t* to)
bool succeed;
if (commit_sizes[i] > 0)
{
succeed = virtual_decommit (commit_begins[i], commit_sizes[i], recorded_committed_bookkeeping_bucket);
succeed = virtual_decommit (28968, commit_begins[i], commit_sizes[i], recorded_committed_bookkeeping_bucket);
assert (succeed);
}
}
Expand Down Expand Up @@ -9419,7 +9420,7 @@ uint32_t* gc_heap::make_card_table (uint8_t* start, uint8_t* end)
// in case of background gc, the mark array will be committed separately (per segment).
size_t commit_size = card_table_element_layout[seg_mapping_table_element + 1];

if (!virtual_commit (mem, commit_size, recorded_committed_bookkeeping_bucket))
if (!virtual_commit (28960, mem, commit_size, recorded_committed_bookkeeping_bucket))
{
dprintf (1, ("Card table commit failed"));
GCToOSInterface::VirtualRelease (mem, alloc_size);
Expand Down Expand Up @@ -9594,7 +9595,7 @@ int gc_heap::grow_brick_card_tables (uint8_t* start,
// in case of background gc, the mark array will be committed separately (per segment).
size_t commit_size = card_table_element_layout[seg_mapping_table_element + 1];

if (!virtual_commit (mem, commit_size, recorded_committed_bookkeeping_bucket))
if (!virtual_commit (28961, mem, commit_size, recorded_committed_bookkeeping_bucket))
{
dprintf (GC_TABLE_LOG, ("Table commit failed"));
set_fgm_result (fgm_commit_table, commit_size, uoh_p);
Expand Down Expand Up @@ -11813,7 +11814,7 @@ void gc_heap::clear_region_info (heap_segment* region)
void gc_heap::return_free_region (heap_segment* region)
{
gc_oh_num oh = heap_segment_oh (region);
dprintf(3, ("commit-accounting: from %d to free [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), heap_number));
STRESS_LOG_VA (1, ("commit-accounting: from %d to free [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), heap_number));
{
size_t committed = heap_segment_committed (region) - get_region_start (region);
if (committed > 0)
Expand Down Expand Up @@ -11907,7 +11908,7 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size)
gen_number, true);

gc_oh_num oh = gen_to_oh (gen_number);
dprintf(3, ("commit-accounting: from free to %d [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), heap_number));
STRESS_LOG_VA (1, ("commit-accounting: from free to %d [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), heap_number));
{
size_t committed = heap_segment_committed (region) - get_region_start (region);
if (committed > 0)
Expand Down Expand Up @@ -12274,7 +12275,7 @@ heap_segment* gc_heap::make_heap_segment (uint8_t* new_pages, size_t size, gc_he
0;
#endif //MULTIPLE_HEAPS

if (!virtual_commit (new_pages, initial_commit, oh, h_number))
if (!virtual_commit (28962, new_pages, initial_commit, oh, h_number))
{
return 0;
}
Expand Down Expand Up @@ -12460,7 +12461,7 @@ size_t gc_heap::decommit_heap_segment_pages_worker (heap_segment* seg,
ptrdiff_t size = heap_segment_committed (seg) - page_start;
if (size > 0)
{
bool decommit_succeeded_p = virtual_decommit (page_start, (size_t)size, heap_segment_oh (seg), heap_number);
bool decommit_succeeded_p = virtual_decommit (28963, page_start, (size_t)size, heap_segment_oh (seg), heap_number);
if (decommit_succeeded_p)
{
dprintf (3, ("Decommitting heap segment [%zx, %zx[(%zd)",
Expand Down Expand Up @@ -12501,7 +12502,7 @@ void gc_heap::decommit_heap_segment (heap_segment* seg)

assert (heap_segment_committed (seg) >= page_start);
size_t size = heap_segment_committed (seg) - page_start;
bool decommit_succeeded_p = virtual_decommit (page_start, size, heap_segment_oh (seg), heap_number);
bool decommit_succeeded_p = virtual_decommit (28964, page_start, size, heap_segment_oh (seg), heap_number);

if (decommit_succeeded_p)
{
Expand Down Expand Up @@ -13269,7 +13270,7 @@ void gc_heap::distribute_free_regions()
size_t end_space = heap_segment_committed (region) - aligned_allocated;
if (end_space > 0)
{
virtual_decommit (aligned_allocated, end_space, gen_to_oh (i), hn);
virtual_decommit (28969, aligned_allocated, end_space, gen_to_oh (i), hn);
heap_segment_committed (region) = aligned_allocated;
heap_segment_used (region) = min (heap_segment_used (region), heap_segment_committed (region));
assert (heap_segment_committed (region) > heap_segment_mem (region));
Expand Down Expand Up @@ -15463,7 +15464,7 @@ BOOL gc_heap::grow_heap_segment (heap_segment* seg, uint8_t* high_address, bool*
"Growing heap_segment: %zx high address: %zx\n",
(size_t)seg, (size_t)high_address);

bool ret = virtual_commit (heap_segment_committed (seg), c_size, heap_segment_oh (seg), heap_number, hard_limit_exceeded_p);
bool ret = virtual_commit (28965, heap_segment_committed (seg), c_size, heap_segment_oh (seg), heap_number, hard_limit_exceeded_p);
if (ret)
{
heap_segment_committed (seg) += c_size;
Expand Down Expand Up @@ -24573,7 +24574,7 @@ heap_segment* gc_heap::unlink_first_rw_region (int gen_idx)
dprintf (REGIONS_LOG, ("unlink_first_rw_region on heap: %d gen: %d region: %p", heap_number, gen_idx, heap_segment_mem (region)));

int oh = heap_segment_oh (region);
dprintf(3, ("commit-accounting: from %d to temp [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), this->heap_number));
STRESS_LOG_VA (1, ("commit-accounting: from %d to temp [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), this->heap_number));
#ifdef _DEBUG
size_t committed = heap_segment_committed (region) - get_region_start (region);
if (committed > 0)
Expand Down Expand Up @@ -24610,7 +24611,7 @@ void gc_heap::thread_rw_region_front (int gen_idx, heap_segment* region)
dprintf (REGIONS_LOG, ("thread_rw_region_front on heap: %d gen: %d region: %p", heap_number, gen_idx, heap_segment_mem (region)));

int oh = heap_segment_oh (region);
dprintf(3, ("commit-accounting: from temp to %d [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), this->heap_number));
STRESS_LOG_VA (1, ("commit-accounting: from temp to %d [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), this->heap_number));
#ifdef _DEBUG
size_t committed = heap_segment_committed (region) - get_region_start (region);
assert (heap_segment_heap (region) == nullptr);
Expand Down Expand Up @@ -24752,7 +24753,7 @@ void gc_heap::equalize_promoted_bytes(int condemned_gen_number)
assert (heap_segment_heap (start_region) == nullptr && hp != nullptr);
int oh = heap_segment_oh (start_region);
size_t committed = heap_segment_committed (start_region) - get_region_start (start_region);
dprintf(3, ("commit-accounting: from temp to %d [%p, %p) for heap %d", oh, get_region_start (start_region), heap_segment_committed (start_region), hp->heap_number));
STRESS_LOG_VA (1, ("commit-accounting: from temp to %d [%p, %p) for heap %d", oh, get_region_start (start_region), heap_segment_committed (start_region), hp->heap_number));
#ifdef _DEBUG
g_heaps[hp->heap_number]->committed_by_oh_per_heap[oh] += committed;
#endif //_DEBUG
Expand Down Expand Up @@ -26070,7 +26071,7 @@ bool gc_heap::change_heap_count (int new_n_heaps)
size_t committed = heap_segment_committed (region) - get_region_start (region);
if (committed > 0)
{
dprintf(3, ("commit-accounting: from %d to %d [%p, %p) for heap %d to heap %d", oh, oh, get_region_start (region), heap_segment_committed (region), i, dest_heap_number));
STRESS_LOG_VA (1, ("commit-accounting: from %d to %d [%p, %p) for heap %d to heap %d", oh, oh, get_region_start (region), heap_segment_committed (region), i, dest_heap_number));
#ifdef _DEBUG
assert (hp->committed_by_oh_per_heap[oh] >= committed);
hp->committed_by_oh_per_heap[oh] -= committed;
Expand Down Expand Up @@ -37731,7 +37732,7 @@ BOOL gc_heap::commit_mark_array_by_range (uint8_t* begin, uint8_t* end, uint32_t
size));
#endif //SIMPLE_DPRINTF

if (virtual_commit (commit_start, size, recorded_committed_mark_array_bucket))
if (virtual_commit (28966, commit_start, size, recorded_committed_mark_array_bucket))
{
// We can only verify the mark array is cleared from begin to end, the first and the last
// page aren't necessarily all cleared 'cause they could be used by other segments or
Expand Down Expand Up @@ -37956,7 +37957,7 @@ void gc_heap::decommit_mark_array_by_seg (heap_segment* seg)

if (decommit_start < decommit_end)
{
if (!virtual_decommit (decommit_start, size, recorded_committed_mark_array_bucket))
if (!virtual_decommit (28970, decommit_start, size, recorded_committed_mark_array_bucket))
{
dprintf (GC_TABLE_LOG, ("decommit on %p for %zd bytes failed",
decommit_start, size));
Expand Down Expand Up @@ -44167,7 +44168,7 @@ size_t gc_heap::decommit_region (heap_segment* region, int bucket, int h_number)
uint8_t* page_start = align_lower_page (get_region_start (region));
uint8_t* decommit_end = heap_segment_committed (region);
size_t decommit_size = decommit_end - page_start;
bool decommit_succeeded_p = virtual_decommit (page_start, decommit_size, bucket, h_number);
bool decommit_succeeded_p = virtual_decommit (28971, page_start, decommit_size, bucket, h_number);
bool require_clearing_memory_p = !decommit_succeeded_p || use_large_pages_p;
dprintf (REGIONS_LOG, ("decommitted region %p(%p-%p) (%zu bytes) - success: %d",
region,
Expand Down Expand Up @@ -47495,10 +47496,6 @@ void gc_heap::verify_committed_bytes_per_heap()

void gc_heap::verify_committed_bytes()
{
#ifndef USE_REGIONS
// TODO, https://github.com/dotnet/runtime/issues/102706, re-enable the testing after fixing this bug
return;
#endif //!USE_REGIONS
size_t total_committed = 0;
size_t committed_decommit; // unused
size_t committed_free; // unused
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/gc/gcpriv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ inline void FATAL_GC_ERROR()
// to the mmap() calls in unix/gcenv.unix.cpp More details here:
// https://github.com/dotnet/runtime/issues/104211
#if defined (HOST_64BIT) && !defined (BUILD_AS_STANDALONE) && !defined(__APPLE__) && !defined(__sun)
#define USE_REGIONS
// #define USE_REGIONS
#endif //HOST_64BIT && BUILD_AS_STANDALONE && !__APPLE__

//#define SPINLOCK_HISTORY
Expand Down Expand Up @@ -2393,9 +2393,9 @@ class gc_heap
#endif //USE_REGIONS
PER_HEAP_METHOD void decommit_heap_segment (heap_segment* seg);
PER_HEAP_ISOLATED_METHOD bool virtual_alloc_commit_for_heap (void* addr, size_t size, int h_number);
PER_HEAP_ISOLATED_METHOD bool virtual_commit (void* address, size_t size, int bucket, int h_number=-1, bool* hard_limit_exceeded_p=NULL);
PER_HEAP_ISOLATED_METHOD bool virtual_decommit (void* address, size_t size, int bucket, int h_number=-1);
PER_HEAP_ISOLATED_METHOD void reduce_committed_bytes (void* address, size_t size, int bucket, int h_number, bool decommit_succeeded_p);
PER_HEAP_ISOLATED_METHOD bool virtual_commit (int reason, void* address, size_t size, int bucket, int h_number=-1, bool* hard_limit_exceeded_p=NULL);
PER_HEAP_ISOLATED_METHOD bool virtual_decommit (int reason, void* address, size_t size, int bucket, int h_number=-1);
PER_HEAP_ISOLATED_METHOD void reduce_committed_bytes (int reason, void* address, size_t size, int bucket, int h_number, bool decommit_succeeded_p);
friend void destroy_card_table (uint32_t*);
PER_HEAP_ISOLATED_METHOD void destroy_card_table_helper (uint32_t* c_table);
PER_HEAP_ISOLATED_METHOD void virtual_free (void* add, size_t size, heap_segment* sg=NULL);
Expand Down
13 changes: 13 additions & 0 deletions src/tests/Common/CLRTest.Execute.Batch.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,15 @@ if not defined CLRTestExecutionArguments (set CLRTestExecutionArguments=$(CLRTes
]]></BatchCLRTestArgPrep>

<!-- By default, be prepared to do a full check -->
<BatchLoopPart2 Condition="'$(CLRTestLoop)' == 'true'"><![CDATA[SET /A RetryCount=%RetryCount%-1
IF NOT "%RetryCount%"=="0" (
GOTO :RetryingRerun
) ELSE (
ECHO END EXECUTION - PASSED BUT ALSO FAILED
ECHO FAILED
popd
Exit /b 1
)]]></BatchLoopPart2>
<BatchCLRTestExitCodeCheck><![CDATA[
ECHO Expected: %CLRTestExpectedExitCode%
ECHO Actual: %CLRTestExitCode%
Expand All @@ -157,6 +166,7 @@ IF NOT "%CLRTestExitCode%"=="%CLRTestExpectedExitCode%" (
) ELSE (
ECHO END EXECUTION - PASSED
ECHO PASSED
$(BatchLoopPart2)
popd
Exit /b 0
)
Expand Down Expand Up @@ -305,6 +315,8 @@ REM Local CoreShim requested - see MSBuild property 'CLRTestScriptLocalCoreShim'
ECHO Copying '%CORE_ROOT%\CoreShim.dll'...
COPY /y %CORE_ROOT%\CoreShim.dll .
]]></BatchCopyCoreShimLocalCmds>
<BatchLoopPart1 Condition="'$(CLRTestLoop)' == 'true'"><![CDATA[SET RetryCount=100
:RetryingRerun]]></BatchLoopPart1>
<BatchCLRTestLaunchCmds Condition="'$(CLRTestKind)' == 'BuildAndRun' And $(TargetOS) != 'android'">
<![CDATA[
$(BatchLinkerTestLaunchCmds)
Expand All @@ -323,6 +335,7 @@ if defined RunCrossGen2 (
)
ECHO %LAUNCHER% %ExePath% %CLRTestExecutionArguments%
$(BatchLoopPart1)
%LAUNCHER% %ExePath% %CLRTestExecutionArguments%
set CLRTestExitCode=!ERRORLEVEL!
if defined RunCrossGen2 (
Expand Down
2 changes: 2 additions & 0 deletions src/tests/Common/Coreclr.TestWrapper/CoreclrTestWrapperLib.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,7 @@ public int RunTest(string executable, string outputFile, string errorFile, strin
using (var errorWriter = new StreamWriter(errorStream))
using (Process process = new Process())
{
outputWriter.WriteLine("Andrew modified CoreclrTestWrapper is running '" + executable + "'");
if (MobileAppHandler.IsRetryRequested(testBinaryBase))
{
outputWriter.WriteLine("\nWork item retry had been requested earlier - skipping test...");
Expand Down Expand Up @@ -719,6 +720,7 @@ public int RunTest(string executable, string outputFile, string errorFile, strin
Task copyOutput = process.StandardOutput.BaseStream.CopyToAsync(outputStream, 4096, cts.Token);
Task copyError = process.StandardError.BaseStream.CopyToAsync(errorStream, 4096, cts.Token);


if (process.WaitForExit(timeout))
{
// Process completed. Check process.ExitCode here.
Expand Down
Loading
Loading