Conversation
gbaraldi
requested review from
topolarity
and removed request for
d-netto,
qinsoon and
udesou
topolarity
added
backport 1.12
|
Despite being featureful, this is a low-risk change that we'd like to have available on 1.12 so I have marked it for backport. |
|
I wonder why you decided to give ability to disable forced FULL GCs instead of just providing a tuning knob to tune when full GC is forced? |
| gc_record_full_sweep_reason(FULL_SWEEP_REASON_FORCED_FULL_SWEEP); | ||
| } | ||
| if (gc_disable_auto_full_sweep && collection != JL_GC_FULL) { | ||
| sweep_full = 0; |
There was a problem hiding this comment.
please take a note - we are monitoring content of gc_record_full_sweep_reason and gc_num during pre/post gc hooks to understand what runtime decided to do.
please make sure that we would be able to reconstruct that FULL GC scheduled but cancelled
There was a problem hiding this comment.
@gbaraldi
Hi Gabriel
I've noticed that you merged this PR without answering question above.
can you reply to that one?
To illustrate issue I've prepared small repo with two demo files that I've collected:
https://github.com/tz-lom/gctest
you can see that with auto_full_collection enabled it is impossible to tell that it FULL GC was not done (except from timing which is much lower, but that is not reliable source of knowledge).
Do I miss something or can I distinguish it differently?
On top of that, I think it would be nice to expose this interface to Julia too, we are controlling GC from Julia most of the time (as GC strategy varies per interface call)
Please reply
There was a problem hiding this comment.
Well, if you disabled full sweeps manually I guess you can query that for your instrumentation. (It's pretty tricky because the main heuristics always change the next GC. Not the current one). Also hypothetically, knowing that the GC would've run a full sweep but didn't because it was manually disabled can be useful to figure out if, when possible, a manual full GC should be ran.
As for the API, I'm slightly wary about adding a Julia level API for this to be back ported. If we like this feature we can add it for non released versions of Julia, but on 1.12 I feel more comfortable keeping this something one has to look for and ccall. In case we decide to go for a different API
Not sure exactly what you mean. This makes it so if the user manually asks for a full GC we still do it, we just don't do it automatically. I don't believe exposing any tuning knobs here is a good idea. |
I meant that there are two constants to control when Full GC is forced, instead of a flag to opt-out from full GC it could be just variable instead of hardcoded constant. |
Keeping this a ccall before adding a julia level API so we can back out if wanted later (cherry picked from commit 370a2a0)
Keeping this a ccall before adding a julia level API so we can back out if wanted later (cherry picked from commit 370a2a0)
Keeping this a ccall before adding a julia level API so we can back out if wanted later
4 participants
Keeping this a ccall before adding a julia level API so we can back out if wanted later