Delay marking of finished tasks

[vchuravy]

Alternative proposal to #47829 intending to fix #40626

The idea is to skip non-runnable tasks during the inital
roots gathering phase. If someone has a reference to a finished
task we won't mess with the task fields.

After we are done with the primary marking phase, we check the
finished tasks and if they have not yet been marked we
set some of the fields we care about to nothing and then
enqueue the other fields.

I briefly thought about not enqueueing the other fields,
but thats much more invasive.

@JeffBezanson One field I would like to zero is the current
stkbuf, but that might be one we actually have to use?

[gbaraldi]

So it turns the fields of the tasks into a kind of weak reference?

[vchuravy]

So it turns the fields of the tasks into a kind of weak reference?

Not quite, it turns the current_task field in ptls into a weak reference, if it is pointing to a task that is not runnable.

[d-netto]

It would be nice to have a test like the one @NHDaly suggested in #47829.

[vchuravy]

@nanosoldier runtests()

[vchuravy]

The problem is that the tests are very indirect. The issue is that ptls->current_task keeps a reference to a done task, thus increasing the life-time of objects referenced by that task.

function foo(x::Vector)
    @async length(x)
end
a = []
r = WeakRef(a)
t = foo(a)
a = nothing
wait(t)
GC.gc()
@test r.value === nothing

Would have worked in my previous attempt, but does not test the issue at hand since t won't be a ptls->current_Task.

[nanosoldier]

Your package evaluation job has completed - possible new issues were detected.
A full report can be found here.

[vtjnash]

This seems possibly not quite sound yet, since one deferred-marked task might refer to another task, making it so you cannot sweep and mark at the same time

[NHDaly]

This seems possibly not quite sound yet, since one deferred-marked task might refer to another task, making it so you cannot sweep and mark at the same time

@vtjnash: If the two tasks you mention here are both non-runnable, meaning that they are only referenced through what we now consider a weak-reference, doesn't it not matter if one references the other? They can both be collected now since the only reference to them is weak?

I'm also not following what you mean by "sweep and mark at the same time" - is that specific to the implementation details in the PR? I didn't read it closely yet.

[apieum]

The problem is that the tests are very indirect. The issue is that ptls->current_task keeps a reference to a done task, thus increasing the life-time of objects referenced by that task.

function foo(x::Vector)
    @async length(x)
end
a = []
r = WeakRef(a)
t = foo(a)
a = nothing
wait(t)
GC.gc()
@test r.value === nothing

Would have worked in my previous attempt, but does not test the issue at hand since t won't be a ptls->current_Task.

This wouldn't work due to weakref behavior (bug ?). If you get value from a weakref, it keeps it.

a = []
r = WeakRef(a)
r.value #if you comment this line the tests pass
a = nothing
GC.gc()
@test a === nothing # this one passes
@test r.value === nothing # this one fails

weakref: Test Failed at ...
   Expression: r.value === nothing
   Evaluated: Any[] === nothing

[quinnj]

Any update on this? Any possible resolution to @vtjnash's concern @vchuravy?

[vchuravy]

I currently don't have the availability to continue work on this.