Skip to content

[release/7.0] fix lower heap hard limit condition for regions - #76588

Merged
carlossanlop merged 8 commits into
release/7.0from
backport/pr-76407-to-release/7.0
Oct 5, 2022
Merged

[release/7.0] fix lower heap hard limit condition for regions#76588
carlossanlop merged 8 commits into
release/7.0from
backport/pr-76407-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Oct 4, 2022

Copy link
Copy Markdown
Contributor

Backport of #76407 to release/7.0

/cc @mangod9

Customer Impact

This fixes a customer reported issue here: #76199, where in the docker config specifies a memory limit (something under 512mb) but with unbound cpu, the GC correctly scales down the number of heaps, but each heap still has a minimum size of 4mb (region_size) * 19 (min_regions_per_heap). This causes an initialization failure since the memory reservation size is 2 * heap_hard_limit. We have now increased this to 5x and also dynamically adjust region size to 4, 2 or 1mb. With this change we are now at par with segments and support very low heap_hard_limits (4mb minimum, which is able to run a simple webapi app)

Testing

validated in local experiments that a heap_hard_limit of as low as 4mb can now be specified, which is similar to what it was with segments. We also adjusted the reservation size when per heap hard_limit is specified

Risk

low, only should affect scenarios where memory hard limit is low. It only affects reservation size, but actual committed sizes should be equivalent.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

@ghostghost added the area-GC-coreclr label Oct 4, 2022
@ghost

ghost commented Oct 4, 2022

Copy link
Copy Markdown

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

Issue Details

Backport of #76407 to release/7.0

/cc @mangod9

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:github-actions[bot]
Assignees:-
Labels:

area-GC-coreclr

Milestone:-

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved. please get a code review and we can take for consideration for 7 ga.

@jeffschwMSFTjeffschwMSFT added the Servicing-consider Issue for next servicing release review label Oct 4, 2022
@jeffschwMSFTjeffschwMSFT added this to the 7.0.0 milestone Oct 4, 2022
@rbhandarbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Oct 4, 2022
oversight from the main PR.
@carlossanlop

Copy link
Copy Markdown
Contributor

Approved, signed-off, CI is green. Ready to merge. :shipit:

@carlossanlop
carlossanlop merged commit 78d8ce7 into release/7.0Oct 5, 2022
@carlossanlop
carlossanlop deleted the backport/pr-76407-to-release/7.0 branch October 5, 2022 17:57
@ghostghost locked as resolved and limited conversation to collaborators Nov 4, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-GC-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@carlossanlop@jeffschwMSFT@Maoni0@rbhanda@mangod9