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.

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

regions BGC work #46972

Merged
merged 1 commit into from
Jan 15, 2021
Merged

regions BGC work #46972

merged 1 commit into from
Jan 15, 2021

Conversation

Maoni0
Copy link
Member

@Maoni0 Maoni0 commented Jan 14, 2021

Regions work (mostly for BGC) -

  • updated the regions list for newly allocated UOH segs, when we delete segs during sweep/compact UOH
    and deleting segs that were marked eligible for deletion during BGC.
  • updated the region list for SOH segs deleted during BGC.
  • commit mark array for new segs if needed for BGC.
  • need to have BGC's ephemeral sweep look at background_allocated of gen0 regions because we could have
    alloc contexts on some gen0 regions that haven't been fixed up yet so we can't go beyond
    background_allocated. and it needs to set it to 0 when it's done with a region so BGC sweep doesn't
    try to look at it.
  • fixed a bug in GCHeap::IsPromoted where we should return true if it's not in condemned.
  • added the necessarily handle table methods to make STRESS_REGIONS work with embedded GC
  • made some dprintfs that only printed region print heap_segment_mem (region) so it's easier to correlate
  • fixed a bug in process_last_np_surv_region that Peter hit when he wasn't using STRESS_REGIONS where we
    ran out gen0 regions for the new consing_gen alloc context. I'm just getting a new region here and assuming
    we can but we need to make sure we actually can (I've put TODOs for this in the code).

Other work -

  • define VERIFY_HEAP for standalone GC and sample.
  • fixed a bug that also exists for doubly linked FL where we didn't update current_sweep_pos when a GC is
    triggered when we are waiting to acquire the UOH alloc lock which caused us to erroneously set BGC mark
    bit during BGC planning.

=============

I've been running this GCPerfSim cmdline for 15 hours with no problems in the following condition -

gcperfsim.dll -tc 2 -tagb 128 -tlgb 0.05 -lohar 100 -sohsi 10 -lohsi 100 -pohsi 0 -sohpi 0 -lohpi 0 -sohfi 0 -lohfi 0 -pohfi 0 -allocType reference -testKind time -printEveryNthIter 300000

with the following env vars -

complus_GCRegionsRange=20000000
complus_GCName=clrgc.dll
(and env vars for logging)

So standalone GC with WKS GC + BGC + STRESS_REGIONS. I actually did do heap verification but a light weight version.

I also tried with embedded GC but just a little to verify the handle table changes.

@ghost
Copy link

ghost commented Jan 14, 2021

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Regions work (mostly for BGC) -

  • updated the regions list for newly allocated UOH segs, when we delete segs during sweep/compact UOH
    and deleting segs that were marked eligible for deletion during BGC.
  • updated the region list for SOH segs deleted during BGC.
  • commit mark array for new segs if needed for BGC.
  • need to have BGC's ephemeral sweep look at background_allocated of gen0 regions because we could have
    alloc contexts on some gen0 regions that haven't been fixed up yet so we can't go beyond
    background_allocated. and it needs to set it to 0 when it's done with a region so BGC sweep doesn't
    try to look at it.
  • fixed a bug in GCHeap::IsPromoted where we should return true if it's not in condemned.
  • added the necessarily handle table methods to make STRESS_REGIONS work with embedded GC
  • made some dprintfs that only printed region print heap_segment_mem (region) so it's easier to correlate
  • fixed a bug in process_last_np_surv_region that Peter hit when he wasn't using STRESS_REGIONS where we
    ran out gen0 regions for the new consing_gen alloc context. I'm just getting a new region here and assuming
    we can but we need to make sure we actually can (I've put TODOs for this in the code).

Other work -

  • define VERIFY_HEAP for standalone GC and sample.
  • fixed a bug that also exists for doubly linked FL where we didn't update current_sweep_pos when a GC is
    triggered when we are waiting to acquire the UOH alloc lock which caused us to erroneously set BGC mark
    bit during BGC planning.

=============

I've been running this GCPerfSim cmdline for 15 hours with no problems in the following condition -

gcperfsim.dll -tc 2 -tagb 128 -tlgb 0.05 -lohar 100 -sohsi 10 -lohsi 100 -pohsi 0 -sohpi 0 -lohpi 0 -sohfi 0 -lohfi 0 -pohfi 0 -allocType reference -testKind time -printEveryNthIter 300000

with the following env vars -

complus_GCRegionsRange=20000000
complus_GCName=clrgc.dll
(and env vars for logging)

So standalone GC with WKS GC + BGC + STRESS_REGIONS. I actually did do heap verification but a light weight version.

I also tried with embedded GC but just a little to verify the handle table changes.

Author: Maoni0
Assignees: -
Labels:

area-GC-coreclr

Milestone: -

@Maoni0
Copy link
Member Author

Maoni0 commented Jan 14, 2021

@PeterSolMS PTAL - you should be able to use STRESS_REGIONS now with coreclr.dll.

@PeterSolMS
Copy link
Contributor

Looks good to me apart from some nits.

Copy link
Contributor

@PeterSolMS PeterSolMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Regions work (mostly for BGC) -

    updated the regions list for newly allocated UOH segs, when we delete segs during sweep/compact UOH
    and deleting segs that were marked eligible for deletion during BGC.
    updated the region list for SOH segs deleted during BGC.
    commit mark array for new segs if needed for BGC.
    need to have BGC's ephemeral sweep look at background_allocated of gen0 regions because we could have
    alloc contexts on some gen0 regions that haven't been fixed up yet so we can't go beyond
    background_allocated. and it needs to set it to 0 when it's done with a region so BGC sweep doesn't
    try to look at it.
    fixed a bug in GCHeap::IsPromoted where we should return true if it's not in condemned.
    added the necessarily handle table methods to make STRESS_REGIONS work with embedded GC
    made some dprintfs that only printed region print heap_segment_mem (region) so it's easier to correlate
    fixed a bug in process_last_np_surv_region that Peter hit when he wasn't using STRESS_REGIONS where we
    ran out gen0 regions for the new consing_gen alloc context. I'm just getting a new region here and assuming
    we can but we need to make sure we actually can (I've put TODOs for this in the code).

Other work -

    define VERIFY_HEAP for standalone GC and sample.
    fixed a bug that also exists for doubly linked FL where we didn't update current_sweep_pos when a GC is
    triggered when we are waiting to acquire the UOH alloc lock which caused us to erroneously set BGC mark
    bit during BGC planning.

=============

I've been running this GCPerfSim cmdline for 15 hours with no problems in the following condition -

gcperfsim.dll -tc 2 -tagb 128 -tlgb 0.05 -lohar 100 -sohsi 10 -lohsi 100 -pohsi 0 -sohpi 0 -lohpi 0 -sohfi 0 -lohfi 0 -pohfi 0 -allocType reference -testKind time -printEveryNthIter 300000

with the following env vars -

complus_GCRegionsRange=20000000
complus_GCName=clrgc.dll
(and env vars for logging)

So standalone GC with WKS GC + BGC + STRESS_REGIONS. I actually did do heap verification but a light weight version.

I also tried with embedded GC but just a little to verify the handle table changes.
@Maoni0 Maoni0 merged commit 0fb9c00 into dotnet:master Jan 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants