Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Suspension fixes #27578

Merged
merged 11 commits into from
Nov 4, 2019
Merged

Suspension fixes #27578

merged 11 commits into from
Nov 4, 2019

Conversation

PeterSolMS
Copy link

Two simple fixes to suspension issues seen in GCPerfSim:

  • Insert allow_fgc() call in background_mark_simple - this fixes the cases where there are a ton of GC handles referencing simple objects not containing pointers.

  • Change PING_JIT_TIMEOUT constant from 10 milliseconds to 1 millisecond. This fixes the case where return address hijacking doesn't work quickly, because the hijacked thread doesn't return (e.g. because it's in a loop doing further calls). In this case we have to retry the hijack, and changing the timeout constant makes this happen more quickly.

…d_first_object when finding the start of objects for marking interior pointers.
…t in find_first_object when finding the start of objects for marking interior pointers."

This reverts commit 9d53ff9.
- Insert allow_fgc() call in background_mark_simple - this fixes the cases where there are a ton of GC handles referencing simple objects not containing pointers.
- Change PING_JIT_TIMEOUT constant from 10 milliseconds to 1 millisecond. This fixes the case where return address hijacking doesn't work quickly, because the hijacked thread doesn't return (e.g. because it's in a loop doing further calls). In this case we have to retry the hijack, and changing the timeout constant makes this happen more quickly.
@Maoni0
Copy link
Member

Maoni0 commented Nov 1, 2019

the other fix that I mentioned that also affects suspension is in revisit_written_page with the same problem in background_mark_simple when we are looking at many pages with no objects that have references in them so we don't have places that allow FGCs. the fix is at the end of this method we add

    if (concurrent_p)
    {
        allow_fgc();
    }

however there is a subtle part of this fix which is if we do allow FGC we will need to get rid of this check in the else if statement:

#ifndef FEATURE_USE_SOFTWARE_WRITE_WATCH_FOR_GC_HEAP // see comment below
                large_objects_p &&
#endif // !FEATURE_USE_SOFTWARE_WRITE_WATCH_FOR_GC_HEAP

because we'd hit the situation that's described in that big comment (essentially what it says is just that since now an FGC can happen it can allocate in free objects so we cannot skip). this change actually has no effect as FEATURE_USE_SOFTWARE_WRITE_WATCH_FOR_GC_HEAP is always defined.

@jkotas jkotas added the area-GC label Nov 2, 2019
@PeterSolMS PeterSolMS merged commit fab7aa2 into dotnet:master Nov 4, 2019
MichalStrehovsky pushed a commit to MichalStrehovsky/corert that referenced this pull request Mar 28, 2020
* Two simple fixes to suspension issues seen in GCPerfSim:

- Insert allow_fgc() call in background_mark_simple - this fixes the cases where there are a ton of GC handles referencing simple objects not containing pointers.

- Change PING_JIT_TIMEOUT constant from 10 milliseconds to 1 millisecond. This fixes the case where return address hijacking doesn't work quickly, because the hijacked thread doesn't return (e.g. because it's in a loop doing further calls). In this case we have to retry the hijack, and changing the timeout constant makes this happen more quickly.

Commit migrated from dotnet/coreclr@fab7aa2
MichalStrehovsky pushed a commit to MichalStrehovsky/corert that referenced this pull request Mar 31, 2020
* Two simple fixes to suspension issues seen in GCPerfSim:

- Insert allow_fgc() call in background_mark_simple - this fixes the cases where there are a ton of GC handles referencing simple objects not containing pointers.

- Change PING_JIT_TIMEOUT constant from 10 milliseconds to 1 millisecond. This fixes the case where return address hijacking doesn't work quickly, because the hijacked thread doesn't return (e.g. because it's in a loop doing further calls). In this case we have to retry the hijack, and changing the timeout constant makes this happen more quickly.

Commit migrated from dotnet/coreclr@fab7aa2
jkotas pushed a commit to dotnet/corert that referenced this pull request Apr 1, 2020
* Two simple fixes to suspension issues seen in GCPerfSim:

- Insert allow_fgc() call in background_mark_simple - this fixes the cases where there are a ton of GC handles referencing simple objects not containing pointers.

- Change PING_JIT_TIMEOUT constant from 10 milliseconds to 1 millisecond. This fixes the case where return address hijacking doesn't work quickly, because the hijacked thread doesn't return (e.g. because it's in a loop doing further calls). In this case we have to retry the hijack, and changing the timeout constant makes this happen more quickly.

Commit migrated from dotnet/coreclr@fab7aa2
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants