From 3d639f216f279e03098a1a5ba93fe764c370c281 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Fri, 21 Aug 2020 14:57:06 +0200 Subject: [PATCH 1/4] delete backport.yml so I can push to master --- .github/workflows/backport.yml | 26 ------------------------ .gitignore | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 26 deletions(-) delete mode 100644 .github/workflows/backport.yml diff --git a/.github/workflows/backport.yml b/.github/workflows/backport.yml deleted file mode 100644 index 79cde1d3ce293f..00000000000000 --- a/.github/workflows/backport.yml +++ /dev/null @@ -1,26 +0,0 @@ -name: Backport PR to branch -on: - issue_comment: - types: [created] - -jobs: - backport: - if: github.event.issue.pull_request != '' && contains(github.event.comment.body, '/backport to') - runs-on: ubuntu-20.04 - steps: - - name: Checkout repo - uses: actions/checkout@v2 - - name: Run backport - uses: ./eng/actions/backport - with: - auth_token: ${{ secrets.GITHUB_TOKEN }} - pr_description_template: | - Backport of #%source_pr_number% to %target_branch% - - /cc %cc_users% - - ## Customer Impact - - ## Testing - - ## Risk diff --git a/.gitignore b/.gitignore index 7596f954d9512a..0a77336126eb44 100644 --- a/.gitignore +++ b/.gitignore @@ -356,3 +356,40 @@ src/coreclr/src/System.Private.CoreLib/common # Mono Wasm-specific excludes src/mono/wasm/emsdk/ src/mono/wasm/.stamp-wasm-install-and-select* +/src/tests/readytorun/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt +/src/tests/readytorun/out/build/x64-Debug (Standard)/rules.ninja +/src/tests/readytorun/out/build/x64-Debug (Standard)/build.ninja +/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply/target-nativelibrary-Debug-db3cc60a4a30aa6ad4f9.json +/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply/index-2020-08-21T10-11-45-0169.json +/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply/codemodel-v2-54a6f181cd4cae4a23f1.json +/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply/cmakeFiles-v1-721ff5cc9aabe9506246.json +/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply/cache-v2-94dde366ed96f1844532.json +/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply +/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json +/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS +/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1 +/src/tests/readytorun/out/build/x64-Debug (Standard) +/src/tests/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt +/src/tests/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json +/src/tests/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS +/src/tests/out/build/x64-Debug (Standard) +/src/tests +/src/mono/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt +/src/mono/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json +/src/mono/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS +/src/mono/out/build/x64-Debug (Standard) +/src/libraries/Native/Windows/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt +/src/libraries/Native/Windows/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json +/src/libraries/Native/Windows/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS +/src/libraries/Native/Windows/out/build/x64-Debug (Standard) +/src/libraries/Native/Unix/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt +/src/libraries/Native/Unix/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json +/src/libraries/Native/Unix/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS +/src/libraries/Native/Unix/out/build/x64-Debug (Standard) +/src/libraries/Native +/src/installer/corehost/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt +/src/installer/corehost/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json +/src/installer/corehost/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS +/src/installer/corehost/out/build/x64-Debug (Standard) +/src/coreclr/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt +/src/coreclr/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json From fb423e872ea3d7c9e0d47234ac4727a55737a423 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 27 Aug 2020 10:56:22 +0200 Subject: [PATCH 2/4] Fix for issue 41311 - Assert failure ephemeral_heap_segment->saved_committed == heap_segment_committed This is an interaction problem between gradual decommit and entering a no GC region. Details: - When we enter a no GC region, we are manipulating our commit goals outside of GC, we usually are planning to commit more, so it makes sense to stop any gradual decommit in progress. - Even if we need to do a GC in preparation for a no GC region, any decommit target established by decommit_ephemeral_segment_pages is likely to be invalid, so we do an early out. - add an assert to grow_heap_segment so we can catch future issues in this area better. We may want to trigger the gradual decommit logic in set_allocations_for_no_gc if we discover that after accounting for the planned allocation, we still have substantial decommits. I'm leaving this for a later PR though. --- src/coreclr/src/gc/gc.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/coreclr/src/gc/gc.cpp b/src/coreclr/src/gc/gc.cpp index c0775bbf982948..2c3629cbafd161 100644 --- a/src/coreclr/src/gc/gc.cpp +++ b/src/coreclr/src/gc/gc.cpp @@ -5703,6 +5703,7 @@ void gc_heap::gc_thread_function () END_TIMING(suspend_ee_during_log); proceed_with_gc_p = TRUE; + gradual_decommit_in_progress_p = FALSE; if (!should_proceed_with_gc()) { @@ -11504,6 +11505,12 @@ BOOL gc_heap::grow_heap_segment (heap_segment* seg, uint8_t* high_address, bool* assert (heap_segment_committed (seg) <= heap_segment_reserved (seg)); assert (high_address <= heap_segment_committed (seg)); + +#ifdef MULTIPLE_HEAPS + assert (!gradual_decommit_in_progress_p || + heap_segment_decommit_target (seg) == nullptr || + heap_segment_committed (seg) <= heap_segment_decommit_target (seg)); +#endif // MULTIPLE_HEAPS } return !!ret; @@ -32456,7 +32463,7 @@ void gc_heap::trim_youngest_desired_low_memory() void gc_heap::decommit_ephemeral_segment_pages() { - if (settings.concurrent || use_large_pages_p) + if (settings.concurrent || use_large_pages_p || settings.pause_mode == pause_no_gc) { return; } @@ -32495,7 +32502,7 @@ void gc_heap::decommit_ephemeral_segment_pages() decommit_target += target_decrease * 2 / 3; } - heap_segment_decommit_target(ephemeral_heap_segment) = decommit_target; + heap_segment_decommit_target (ephemeral_heap_segment) = decommit_target; #ifdef MULTIPLE_HEAPS if (decommit_target < heap_segment_committed (ephemeral_heap_segment)) From 0fc9aebbf6d85d57e79c7ef5814e4fd1fec35813 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 27 Aug 2020 11:36:14 +0200 Subject: [PATCH 3/4] Undo unintended changes --- .github/workflows/backport.yml | 26 ++++++++++++++++++++++++ .gitignore | 37 ---------------------------------- 2 files changed, 26 insertions(+), 37 deletions(-) create mode 100644 .github/workflows/backport.yml diff --git a/.github/workflows/backport.yml b/.github/workflows/backport.yml new file mode 100644 index 00000000000000..79cde1d3ce293f --- /dev/null +++ b/.github/workflows/backport.yml @@ -0,0 +1,26 @@ +name: Backport PR to branch +on: + issue_comment: + types: [created] + +jobs: + backport: + if: github.event.issue.pull_request != '' && contains(github.event.comment.body, '/backport to') + runs-on: ubuntu-20.04 + steps: + - name: Checkout repo + uses: actions/checkout@v2 + - name: Run backport + uses: ./eng/actions/backport + with: + auth_token: ${{ secrets.GITHUB_TOKEN }} + pr_description_template: | + Backport of #%source_pr_number% to %target_branch% + + /cc %cc_users% + + ## Customer Impact + + ## Testing + + ## Risk diff --git a/.gitignore b/.gitignore index 0a77336126eb44..7596f954d9512a 100644 --- a/.gitignore +++ b/.gitignore @@ -356,40 +356,3 @@ src/coreclr/src/System.Private.CoreLib/common # Mono Wasm-specific excludes src/mono/wasm/emsdk/ src/mono/wasm/.stamp-wasm-install-and-select* -/src/tests/readytorun/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt -/src/tests/readytorun/out/build/x64-Debug (Standard)/rules.ninja -/src/tests/readytorun/out/build/x64-Debug (Standard)/build.ninja -/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply/target-nativelibrary-Debug-db3cc60a4a30aa6ad4f9.json -/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply/index-2020-08-21T10-11-45-0169.json -/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply/codemodel-v2-54a6f181cd4cae4a23f1.json -/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply/cmakeFiles-v1-721ff5cc9aabe9506246.json -/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply/cache-v2-94dde366ed96f1844532.json -/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/reply -/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json -/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS -/src/tests/readytorun/out/build/x64-Debug (Standard)/.cmake/api/v1 -/src/tests/readytorun/out/build/x64-Debug (Standard) -/src/tests/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt -/src/tests/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json -/src/tests/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS -/src/tests/out/build/x64-Debug (Standard) -/src/tests -/src/mono/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt -/src/mono/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json -/src/mono/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS -/src/mono/out/build/x64-Debug (Standard) -/src/libraries/Native/Windows/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt -/src/libraries/Native/Windows/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json -/src/libraries/Native/Windows/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS -/src/libraries/Native/Windows/out/build/x64-Debug (Standard) -/src/libraries/Native/Unix/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt -/src/libraries/Native/Unix/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json -/src/libraries/Native/Unix/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS -/src/libraries/Native/Unix/out/build/x64-Debug (Standard) -/src/libraries/Native -/src/installer/corehost/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt -/src/installer/corehost/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json -/src/installer/corehost/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS -/src/installer/corehost/out/build/x64-Debug (Standard) -/src/coreclr/out/build/x64-Debug (Standard)/VSInheritEnvironments.txt -/src/coreclr/out/build/x64-Debug (Standard)/.cmake/api/v1/query/client-MicrosoftVS/query.json From e4503085d4afda61f1c9bee2299cf73a11398d94 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Fri, 28 Aug 2020 10:57:35 +0200 Subject: [PATCH 4/4] Address code review feedback, fix assert. --- src/coreclr/src/gc/gc.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/coreclr/src/gc/gc.cpp b/src/coreclr/src/gc/gc.cpp index c90e69a580aae5..adb9feabd632da 100644 --- a/src/coreclr/src/gc/gc.cpp +++ b/src/coreclr/src/gc/gc.cpp @@ -11509,9 +11509,12 @@ BOOL gc_heap::grow_heap_segment (heap_segment* seg, uint8_t* high_address, bool* assert (high_address <= heap_segment_committed (seg)); #ifdef MULTIPLE_HEAPS + // we should never increase committed beyond decommit target when gradual + // decommit is in progress - if we do, this means commit and decommit are + // going on at the same time. assert (!gradual_decommit_in_progress_p || - heap_segment_decommit_target (seg) == nullptr || - heap_segment_committed (seg) <= heap_segment_decommit_target (seg)); + (seg != ephemeral_heap_segment) || + (heap_segment_committed (seg) <= heap_segment_decommit_target (seg))); #endif // MULTIPLE_HEAPS } @@ -32465,7 +32468,7 @@ void gc_heap::trim_youngest_desired_low_memory() void gc_heap::decommit_ephemeral_segment_pages() { - if (settings.concurrent || use_large_pages_p || settings.pause_mode == pause_no_gc) + if (settings.concurrent || use_large_pages_p || (settings.pause_mode == pause_no_gc)) { return; }