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

Fixes when accessing fgn_maxgen_percent #25650

Merged
2 commits merged into from Jul 12, 2019
Merged

Fixes when accessing fgn_maxgen_percent #25650

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11e9864
Fixes when accessing fgn_maxgen_percent
Jul 11, 2019
c3798cf
Avoid duplicate code when getting fgn_maxgen_percent twice in full_gc…
Jul 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/gc/gc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12377,13 +12377,13 @@ void gc_heap::send_full_gc_notification (int gen_num, BOOL due_to_alloc_p)

wait_full_gc_status gc_heap::full_gc_wait (GCEvent *event, int time_out_ms)
{
uint32_t maxgen_percent = 0;
#ifdef MULTIPLE_HEAPS
maxgen_percent = g_heaps[0]->fgn_maxgen_percent;
gc_heap* hp = gc_heap::g_heaps[0];
#else
maxgen_percent = fgn_maxgen_percent;
gc_heap* hp = pGenGCHeap;
#endif //MULTIPLE_HEAPS
if (maxgen_percent == 0)

if (hp->fgn_maxgen_percent == 0)
{
return wait_full_gc_na;
}
Expand All @@ -12392,7 +12392,7 @@ wait_full_gc_status gc_heap::full_gc_wait (GCEvent *event, int time_out_ms)

if ((wait_result == WAIT_OBJECT_0) || (wait_result == WAIT_TIMEOUT))
{
if (maxgen_percent == 0)
if (hp->fgn_maxgen_percent == 0)
{
Copy link
Member

@Maoni0 Maoni0 Jul 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maxgen_percent = g_heaps[0]->fgn_maxgen_percent; [](start = 7, length = 49)

maxgen_percent was already read at the beginning of this method.

Copy link
Member

@Maoni0 Maoni0 Jul 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see your comment from above - is it because the 1st time we read the value we observe its not set yet?

Copy link
Author

@ghost ghost Jul 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The operation may be canceled while we're waiting, and CancelFullGCNotification works by setting fgn_maxgen_percent.

Copy link
Member

@Maoni0 Maoni0 Jul 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh right; we still wanna return the right status so we need to re-read. makes sense.

Copy link
Member

@Maoni0 Maoni0 Jul 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw we could also write the code this way (like what the other places are doing) so you don't have to duplicate the code:

#ifdef MULTIPLE_HEAPS
    gc_heap* hp = gc_heap::g_heaps[0];
#else
    gc_heap* hp = pGenGCHeap;
#endif //MULTIPLE_HEAPS

if (hp->fgn_maxgen_percent == 0)

return wait_full_gc_cancelled;
}
Expand Down Expand Up @@ -36900,6 +36900,7 @@ bool GCHeap::RegisterForFullGCNotification(uint32_t gen2Percentage,
}
#else //MULTIPLE_HEAPS
pGenGCHeap->fgn_last_alloc = dd_new_allocation (pGenGCHeap->dynamic_data_of (0));
pGenGCHeap->fgn_maxgen_percent = gen2Percentage;
#endif //MULTIPLE_HEAPS

pGenGCHeap->full_gc_approach_event.Reset();
Expand All @@ -36913,9 +36914,17 @@ bool GCHeap::RegisterForFullGCNotification(uint32_t gen2Percentage,

bool GCHeap::CancelFullGCNotification()
{
#ifdef MULTIPLE_HEAPS
for (int hn = 0; hn < gc_heap::n_heaps; hn++)
{
gc_heap* hp = gc_heap::g_heaps [hn];
hp->fgn_maxgen_percent = 0;
}
#else //MULTIPLE_HEAPS
pGenGCHeap->fgn_maxgen_percent = 0;
pGenGCHeap->fgn_loh_percent = 0;
#endif //MULTIPLE_HEAPS

pGenGCHeap->fgn_loh_percent = 0;
pGenGCHeap->full_gc_approach_event.Set();
pGenGCHeap->full_gc_end_event.Set();

Expand Down