gcext: add ptls argument to task scanner #54638
Conversation
|
While I agree that this is a good simplification of the API, I would like to understand the rational for backporting it a bit better. E.g. why isn't |
|
That's a great question and actually I've been debating back and forth with myself whether to submit this in the first place (ignoring backports), and so perhaps this should just be closed outright (sorry, I should have made that clear in the PR description but was too tired last night/morning). So first off, the root scanner actually does That said, it still seems like an oversight to me that we don't pass the ptls to those callbacks, all other GC internals calls get the ptls as an argument. But it may not be worth the pain to do it at all, given that |
|
Yeah at this point in the 1.11 cycle it becomes a risk assessment. Since you are one of the primary users of this interface it's up to you. My concern is that there may already be binaries built against 1.11 and while the change should not be an issue on x86 Linux I am less sure about other ABIs. So LGTM for 1.12 but for 1.11 I would be conservative. |
Access to ptls is generally necessary when marking objects so it makes sense to pass it to the task scanner. We pass it as the last argument to the callback so that existing consumers of this API hopefully keep working, they will just ignore this new argument.
fingolfin
force-pushed
the
mh/task_scanner
branch
from
340f13f to
974a835
Compare
|
With @vchuravy's "LGTM for 1.12", I imagine this could probably be merged? |
Access to ptls is generally necessary when marking objects so it makes sense to pass it to the task scanner.
We pass it as the last argument to the callback so that existing consumers of this API hopefully keep working without recompilation, they will just ignore this new argument.
I hope this can still be put into 1.11.
From my POV it could also go into 1.10, as existing binaries linked again prior Julia 1.10.x releases should keep working; but compiling consumers of this API will trigger a warning or even error (trivial to fix), which seems somewhat problematic to me? So perhaps better not.