Skip to content

Disable GetGCMemoryInfo on arm - #73477

Merged
hoyosjs merged 1 commit into
mainfrom
juhoyosa/disable-gc-getinfo
Aug 8, 2022
Merged

Disable GetGCMemoryInfo on arm#73477
hoyosjs merged 1 commit into
mainfrom
juhoyosa/disable-gc-getinfo

Conversation

@hoyosjs

@hoyosjshoyosjs commented Aug 5, 2022

Copy link
Copy Markdown
Member

Disables test blocking CI #73247

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 5, 2022
@hoyosjs
hoyosjs marked this pull request as ready for review August 5, 2022 17:30
@ghostghost assigned hoyosjsAug 5, 2022
@ghost

ghost commented Aug 5, 2022

Copy link
Copy Markdown

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

Issue Details

Disables test blocking CI #73247

Author:hoyosjs
Assignees:-
Labels:

area-CodeGen-coreclr

Milestone:-

@hoyosjshoyosjs added area-GC-coreclr and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Aug 5, 2022
@ghost

ghost commented Aug 5, 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

Disables test blocking CI #73247

Author:hoyosjs
Assignees:hoyosjs
Labels:

area-GC-coreclr

Milestone:-

@hoyosjshoyosjs mentioned this pull request Aug 5, 2022
Closed
@hoyosjs
hoyosjs requested review from a teamAugust 5, 2022 17:33
@cshung

cshung commented Aug 5, 2022

Copy link
Copy Markdown
Contributor

@hoyosjs, Can we test this fix instead? #73247

@hoyosjs

hoyosjs commented Aug 5, 2022

Copy link
Copy Markdown
MemberAuthor

Seems like a reasonable workaround - is it OK to not have accounting on win-x86?

@hoyosjs

hoyosjs commented Aug 5, 2022•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

I'll leave this open and merge later if the other PR is not approved by the time this CI is done. We can easily add the revert to your PR, but this issue is hitting at a high rate.

@cshung

cshung commented Aug 5, 2022

Copy link
Copy Markdown
Contributor

Seems like a reasonable workaround - is it OK to not have accounting on win-x86?

Yes - heap_hard_limit is supported on 64 bit platforms only

@hoyosjs

hoyosjs commented Aug 5, 2022

Copy link
Copy Markdown
MemberAuthor

Ah, that'd be a proper fix altogether then.

@mangod9

mangod9 commented Aug 5, 2022

Copy link
Copy Markdown
Member

what in that change was causing the ArgumentOutOfRange exception though?

@cshung

cshung commented Aug 5, 2022

Copy link
Copy Markdown
Contributor

what in that change was causing the ArgumentOutOfRange exception though?

The test case itself did it - and that was not a problem at all.

@hoyosjshoyosjs closed this Aug 5, 2022
@hoyosjshoyosjs reopened this Aug 5, 2022
@hoyosjs
hoyosjs merged commit 9865cc7 into mainAug 8, 2022
@hoyosjs
hoyosjs deleted the juhoyosa/disable-gc-getinfo branch August 8, 2022 17:02
@am11am11 mentioned this pull request Aug 8, 2022
Merged
noahfalk added a commit to noahfalk/runtime that referenced this pull request Aug 8, 2022
@noahfalk
Revert "Disable GetGCMemoryInfo on arm (dotnet#73477)"
8694661
This reverts commit 9865cc7.
We believe the problematic change has been reverted so the test can be re-enabled.
hoyosjs pushed a commit that referenced this pull request Aug 9, 2022
@noahfalk
Revert "Disable GetGCMemoryInfo on arm (#73477)" (#73595)
d0aa175
This reverts commit 9865cc7.
We believe the problematic change has been reverted so the test can be re-enabled.
@ghostghost locked as resolved and limited conversation to collaborators Sep 7, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mangod9mangod9mangod9 approved these changes

Assignees

@hoyosjshoyosjs

Labels

area-GC-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hoyosjs@cshung@mangod9