Skip to content

[mono][sgen] Update memory load information after world is resumed - #76400

Merged
akoeplinger merged 1 commit into
dotnet:mainfrom
BrzVlad:fix-cgroup-hang
Oct 3, 2022
Merged

[mono][sgen] Update memory load information after world is resumed#76400
akoeplinger merged 1 commit into
dotnet:mainfrom
BrzVlad:fix-cgroup-hang

Conversation

@BrzVlad

Copy link
Copy Markdown
Member

Following recent cgroup support (#74237), mono_determine_physical_ram_size is no longer safe to call during GC.

Following recent cgroup support, mono_determine_physical_ram_size is no longer safe to call during GC.
@ghost

Copy link
Copy Markdown

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

Issue Details

Following recent cgroup support (#74237), mono_determine_physical_ram_size is no longer safe to call during GC.

Author:BrzVlad
Assignees:-
Labels:

area-GC-mono

Milestone:-

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

@nealef@akoeplinger Any idea if this impacts android ?

@naricc

Copy link
Copy Markdown
Contributor

What part of #74237 makes it unsafe to call mono_determine_physical_ram_size during GC?

@nealef

Copy link
Copy Markdown
Contributor

@nealef@akoeplinger Any idea if this impacts android ?

The new code explicitly excludes Android. That is, it uses the previous code path.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

@nariccgetCGroupMemoryLimit is the main one, it uses a lot of C api's that are unsafe.

@akoeplinger

Copy link
Copy Markdown
Member

I didn't realize we query the ram size on each GC stw cycle, do we know if the cgroup additions have any perf impact?

We'll also need to backport this to 7.0 since we backported the cgroup change.

@naricc

naricc commented Sep 30, 2022

Copy link
Copy Markdown
Contributor

I didn't realize we query the ram size on each GC stw cycle, do we know if the cgroup additions have any perf impact?

I have not seen any perf regressions that I could attribute to this, although the microbenchmarks are so micro I am not sure how many of them exercise the GC much. There was also some missing data on some platforms around the time this was merged.

@akoeplinger

Copy link
Copy Markdown
Member

Failures are unrelated.

@akoeplinger
akoeplinger merged commit a08d300 into dotnet:mainOct 3, 2022
@akoeplinger

Copy link
Copy Markdown
Member

/backport to release/7.0

@github-actions

Copy link
Copy Markdown
Contributor

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

@ghostghost locked as resolved and limited conversation to collaborators Nov 2, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@BrzVlad@naricc@nealef@akoeplinger