This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
Bgc suspension fixes #27729
Merged
Bgc suspension fixes #27729
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
…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.
…GC is in progress: - In background_mark_simple1, check g_fSuspensionPending and if it is set, save the state of the work and restart the loop - this will call allow_fgc() and thus allow a foreground GC to take place. - In revisit_written_page, call allow_fgc() at the end - this allow a foreground GC to happen whenever we are done with revisiting a written page.
instead of checking something like g_fSuspensionPending, it might be better to count the already marked refs like we do with the ones we need to mark; since we do less work with marked refs we can exit the loop say 16 * num_partial_refs. this way we don't need to bring in the g_fSuspensionPending var at all (which would be more elegant 'cause it's supposed to be encapsulate in the allow_fgc logic). something like this: int num_unmarked_refs = num_partial_refs;
int num_marked_refs = num_unmarked_refs * 16;
go_through_object (method_table(oo), oo, s, ppslot,
start, use_start, (oo + s),
{
uint8_t* o = *ppslot;
Prefetch(o);
if (background_mark (o,
background_saved_lowest_address,
background_saved_highest_address))
{
// same code here
}
else if (--num_marked_refs == 0)
{
*place = (uint8_t*)(ppslot + 1);
goto more_to_do;
}
}
);
what do you think? |
I considered this but worried that the additional variable would live on the stack and thus we'd have additional memory references. I'll give it a try... |
…spensionPending directly.
Ok, I tried your counter solution and found that the additional counter lives in a register, not on the stack (in x64 release). Also, in PerfView I could not see a speed difference between the two. I made the following small tweaks to your suggested solution:
|
MichalStrehovsky pushed a commit to MichalStrehovsky/corert that referenced this pull request
Mar 28, 2020
* Changes to set gen0 bricks always. This reduces the time spent in find_first_object when finding the start of objects for marking interior pointers. * Revert "Changes to set gen0 bricks always. This reduces the time spent in find_first_object when finding the start of objects for marking interior pointers." This reverts commit dotnet/coreclr@9d53ff9. * Two fixes to speed up suspension for foreground GCs while background GC is in progress: - In background_mark_simple1, check g_fSuspensionPending and if it is set, save the state of the work and restart the loop - this will call allow_fgc() and thus allow a foreground GC to take place. - In revisit_written_page, call allow_fgc() at the end - this allow a foreground GC to happen whenever we are done with revisiting a written page. * Addressed code review feedback - use counter instead of testing g_fSuspensionPending directly. Commit migrated from dotnet/coreclr@a7678f5
MichalStrehovsky pushed a commit to MichalStrehovsky/corert that referenced this pull request
Mar 31, 2020
* Changes to set gen0 bricks always. This reduces the time spent in find_first_object when finding the start of objects for marking interior pointers. * Revert "Changes to set gen0 bricks always. This reduces the time spent in find_first_object when finding the start of objects for marking interior pointers." This reverts commit dotnet/coreclr@9d53ff9. * Two fixes to speed up suspension for foreground GCs while background GC is in progress: - In background_mark_simple1, check g_fSuspensionPending and if it is set, save the state of the work and restart the loop - this will call allow_fgc() and thus allow a foreground GC to take place. - In revisit_written_page, call allow_fgc() at the end - this allow a foreground GC to happen whenever we are done with revisiting a written page. * Addressed code review feedback - use counter instead of testing g_fSuspensionPending directly. Commit migrated from dotnet/coreclr@a7678f5
jkotas pushed a commit to dotnet/corert that referenced this pull request
Apr 1, 2020
* Changes to set gen0 bricks always. This reduces the time spent in find_first_object when finding the start of objects for marking interior pointers. * Revert "Changes to set gen0 bricks always. This reduces the time spent in find_first_object when finding the start of objects for marking interior pointers." This reverts commit dotnet/coreclr@9d53ff9. * Two fixes to speed up suspension for foreground GCs while background GC is in progress: - In background_mark_simple1, check g_fSuspensionPending and if it is set, save the state of the work and restart the loop - this will call allow_fgc() and thus allow a foreground GC to take place. - In revisit_written_page, call allow_fgc() at the end - this allow a foreground GC to happen whenever we are done with revisiting a written page. * Addressed code review feedback - use counter instead of testing g_fSuspensionPending directly. Commit migrated from dotnet/coreclr@a7678f5
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Two fixes to speed up suspension for foreground GCs while background GC is in progress:
In background_mark_simple1, check g_fSuspensionPending and if it is set, save the state of the work and restart the loop - this will call allow_fgc() and thus allow a foreground GC to take place. Some measurements indicated this would regress performance of background_mark_simple1, but the effect seems to have disappeared when I remeasured. Nonetheless I'm curious if there is a better way.
In revisit_written_page, call allow_fgc() at the end - this allow a foreground GC to happen whenever we are done with revisiting a written page. This change was suggested by Maoni.