Skip to content

fix lower heap hard limit condition for regions - #76407

Merged
mangod9 merged 7 commits into
dotnet:mainfrom
mangod9:update_reserve_size
Oct 4, 2022
Merged

fix lower heap hard limit condition for regions#76407
mangod9 merged 7 commits into
dotnet:mainfrom
mangod9:update_reserve_size

Conversation

@mangod9

@mangod9mangod9 commented Sep 29, 2022•
edited
Loading

Copy link
Copy Markdown
Member

Fixes#76199, where containers < 512mb were failing with E_OOM during GC initialization, since the check for region_size * heaps * 19 (various generations) < reservation size was failing.

The logic is to now dynamically adjust region_size for smaller heap sizes. So for very small heap_hard_limits the region_size would be 1mb (instead of the default 4mb currently)

@ghostghost added the area-GC-coreclr label Sep 29, 2022
@ghostghost assigned mangod9Sep 29, 2022
@ghost

ghost commented Sep 29, 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

Fixes #76199, where containers < 512mb were failing with E_OOM during GC initialization, since the check for region_size * heaps * 19 (various generations) < reservation size was failing

Author:mangod9
Assignees:-
Labels:

area-GC-coreclr

Milestone:-

mangod9
mangod9 commented Sep 29, 2022
View reviewed changes
Comment threadsrc/coreclr/gc/gc.cpp
@mangod9

mangod9 commented Sep 30, 2022

Copy link
Copy Markdown
MemberAuthor

with the latest change, the lowest hard_limit possible is 0x400000 (4mb)

Maoni0
Maoni0 reviewed Oct 3, 2022
View reviewed changes
Comment threadsrc/coreclr/gc/gc.cpp Outdated
Maoni0
Maoni0 reviewed Oct 3, 2022
View reviewed changes
Comment threadsrc/coreclr/gc/gc.cpp Outdated
Maoni0
Maoni0 reviewed Oct 3, 2022
View reviewed changes
Comment threadsrc/coreclr/gc/gc.cpp
Maoni0
Maoni0 approved these changes Oct 3, 2022
View reviewed changes

@Maoni0Maoni0 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.

other than the nit above, it LGTM

@mangod9

mangod9 commented Oct 4, 2022

Copy link
Copy Markdown
MemberAuthor

The Hardware Instrinsics failure looks related to: #76280. Other PRs are failing with that same failure. Need to port to 7 hence merging.

@mangod9
mangod9 merged commit 4ca326f into dotnet:mainOct 4, 2022
@mangod9

mangod9 commented Oct 4, 2022

Copy link
Copy Markdown
MemberAuthor

/backport to release/7.0

@github-actions

github-actionsBot commented Oct 4, 2022

Copy link
Copy Markdown
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3179565569

@github-actionsgithub-actionsBot mentioned this pull request Oct 4, 2022
Merged
@mangod9
mangod9 deleted the update_reserve_size branch October 4, 2022 17:41
@ghostghost locked as resolved and limited conversation to collaborators Nov 3, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Maoni0Maoni0Maoni0 approved these changes
@cshungcshungAwaiting requested review from cshung

Assignees

@mangod9mangod9

Labels

area-GC-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

.NET 7.0.100-preview.7.22377.5 threw an exception in docker (with memory limit but unbounded cpu)

2 participants

@mangod9@Maoni0