Simplify Task structure (#466)

Instead of restarting new Tasks for file-watching, this introduces a 
loop. This seems a little easier to reason about, but this is mostly 
cosmetic. 

Inspired by investigating #459, but this does not fix it.

docs/src/dev_reference.md

@@ -40,7 +40,7 @@ Revise.FileInfo
 Revise.PkgData
 Revise.WatchList
 Revise.SlotDep
-Revise.Rescheduler
+Revise.TaskThunk
 MethodSummary
 ```
 

src/Revise.jl

@@ -456,7 +456,7 @@ function init_watching(pkgdata::PkgData, files)
         already_watching || (watched_files[dirfull] = WatchList())
         push!(watched_files[dirfull], basename=>pkgdata)
         if watching_files[]
-            fwatcher = Rescheduler(revise_file_queued, (pkgdata, file))
+            fwatcher = TaskThunk(revise_file_queued, (pkgdata, file))
             schedule(Task(fwatcher))
         else
             already_watching || push!(udirs, dir)
@@ -466,7 +466,7 @@ function init_watching(pkgdata::PkgData, files)
         dirfull = joinpath(basedir(pkgdata), dir)
         updatetime!(watched_files[dirfull])
         if !watching_files[]
-            dwatcher = Rescheduler(revise_dir_queued, (dirfull,))
+            dwatcher = TaskThunk(revise_dir_queued, (dirfull,))
             schedule(Task(dwatcher))
         end
     end
@@ -478,36 +478,39 @@ end
 
 Wait for one or more of the files registered in `Revise.watched_files[dirname]` to be
 modified, and then queue the corresponding files on [`Revise.revision_queue`](@ref).
-This is generally called via a [`Revise.Rescheduler`](@ref).
+This is generally called via a [`Revise.TaskThunk`](@ref).
 """
 @noinline function revise_dir_queued(dirname)
     @assert isabspath(dirname)
     if !isdir(dirname)
         sleep(0.1)   # in case git has done a delete/replace cycle
+    end
+    stillwatching = true
+    while stillwatching
         if !isdir(dirname)
             with_logger(SimpleLogger(stderr)) do
                 @warn "$dirname is not an existing directory, Revise is not watching"
             end
-            return false
+            break
         end
-    end
-    latestfiles, stillwatching = watch_files_via_dir(dirname)  # will block here until file(s) change
-    for (file, id) in latestfiles
-        key = joinpath(dirname, file)
-        pkgdata = pkgdatas[id]
-        if hasfile(pkgdata, key)  # issue #228
-            push!(revision_queue, (pkgdata, relpath(key, pkgdata)))
+        latestfiles, stillwatching = watch_files_via_dir(dirname)  # will block here until file(s) change
+        for (file, id) in latestfiles
+            key = joinpath(dirname, file)
+            pkgdata = pkgdatas[id]
+            if hasfile(pkgdata, key)  # issue #228
+                push!(revision_queue, (pkgdata, relpath(key, pkgdata)))
+            end
         end
     end
-    return stillwatching
+    return
 end
 
 # See #66.
 """
     revise_file_queued(pkgdata::PkgData, filename)
 
 Wait for modifications to `filename`, and then queue the corresponding files on [`Revise.revision_queue`](@ref).
-This is generally called via a [`Revise.Rescheduler`](@ref).
+This is generally called via a [`Revise.TaskThunk`](@ref).
 
 This is used only on platforms (like BSD) which cannot use [`Revise.revise_dir_queued`](@ref).
 """
@@ -518,20 +521,23 @@ function revise_file_queued(pkgdata::PkgData, file)
     end
     if !file_exists(file)
         sleep(0.1)  # in case git has done a delete/replace cycle
-        if !file_exists(file)
-            push!(revision_queue, (pkgdata, file0))  # process file deletions
-            return false
-        end
     end
 
-    wait_changed(file)  # will block here until the file changes
-    # Check to see if we're still watching this file
     dirfull, basename = splitdir(file)
-    if haskey(watched_files, dirfull)
+    stillwatching = true
+    while stillwatching
+        if !file_exists(file)
+            with_logger(SimpleLogger(stderr)) do
+                @warn "$file is not an existing file, Revise is not watching"
+            end
+            break
+        end
+        wait_changed(file)  # will block here until the file changes
+        # Check to see if we're still watching this file
+        stillwatching = haskey(watched_files, dirfull)
         push!(revision_queue, (pkgdata, file0))
-        return true
     end
-    return false
+    return
 end
 
 # Because we delete first, we have to make sure we've parsed the file
@@ -1155,7 +1161,7 @@ function __init__()
     if mfile === nothing
         @warn "no Manifest.toml file found, static paths used"
     else
-        wmthunk = Rescheduler(watch_manifest, (mfile,))
+        wmthunk = TaskThunk(watch_manifest, (mfile,))
         schedule(Task(wmthunk))
     end
     return nothing

src/pkgs.jl

@@ -480,49 +480,50 @@ function find_from_hash(name, uuid, hash)
 end
 
 function watch_manifest(mfile)
-    wait_changed(mfile)
-    try
-        with_logger(_debug_logger) do
-            @debug "Pkg" _group="manifest_update" manifest_file=mfile
-            isfile(mfile) || return nothing
-            pkgdirs = manifest_paths(mfile)
-            for (id, pkgdir) in pkgdirs
-                if haskey(pkgdatas, id)
-                    pkgdata = pkgdatas[id]
-                    if pkgdir != basedir(pkgdata)
-                        ## The package directory has changed
-                        @debug "Pkg" _group="pathswitch" oldpath=basedir(pkgdata) newpath=pkgdir
-                        # Stop all associated watching tasks
-                        for dir in unique_dirs(srcfiles(pkgdata))
-                            @debug "Pkg" _group="unwatch" dir=dir
-                            delete!(watched_files, joinpath(basedir(pkgdata), dir))
-                            # Note: if the file is revised, the task(s) will run one more time.
-                            # However, because we've removed the directory from the watch list this will be a no-op,
-                            # and then the tasks will be dropped.
-                        end
-                        # Revise code as needed
-                        files = String[]
-                        for file in srcfiles(pkgdata)
-                            maybe_parse_from_cache!(pkgdata, file)
-                            push!(revision_queue, (pkgdata, file))
-                            push!(files, file)
-                        end
-                        # Update the directory
-                        pkgdata.info.basedir = pkgdir
-                        # Restart watching, if applicable
-                        if has_writable_paths(pkgdata)
-                            init_watching(pkgdata, files)
+    while true
+        wait_changed(mfile)
+        try
+            with_logger(_debug_logger) do
+                @debug "Pkg" _group="manifest_update" manifest_file=mfile
+                isfile(mfile) || return nothing
+                pkgdirs = manifest_paths(mfile)
+                for (id, pkgdir) in pkgdirs
+                    if haskey(pkgdatas, id)
+                        pkgdata = pkgdatas[id]
+                        if pkgdir != basedir(pkgdata)
+                            ## The package directory has changed
+                            @debug "Pkg" _group="pathswitch" oldpath=basedir(pkgdata) newpath=pkgdir
+                            # Stop all associated watching tasks
+                            for dir in unique_dirs(srcfiles(pkgdata))
+                                @debug "Pkg" _group="unwatch" dir=dir
+                                delete!(watched_files, joinpath(basedir(pkgdata), dir))
+                                # Note: if the file is revised, the task(s) will run one more time.
+                                # However, because we've removed the directory from the watch list this will be a no-op,
+                                # and then the tasks will be dropped.
+                            end
+                            # Revise code as needed
+                            files = String[]
+                            for file in srcfiles(pkgdata)
+                                maybe_parse_from_cache!(pkgdata, file)
+                                push!(revision_queue, (pkgdata, file))
+                                push!(files, file)
+                            end
+                            # Update the directory
+                            pkgdata.info.basedir = pkgdir
+                            # Restart watching, if applicable
+                            if has_writable_paths(pkgdata)
+                                init_watching(pkgdata, files)
+                            end
                         end
                     end
                 end
             end
-        end
-    catch err
-        @static if VERSION >= v"1.2.0-DEV.253"
-            put!(Base.active_repl_backend.response_channel, (Base.catch_stack(), true))
-        else
-            put!(Base.active_repl_backend.response_channel, (err, catch_backtrace()))
+        catch err
+            @static if VERSION >= v"1.2.0-DEV.253"
+                put!(Base.active_repl_backend.response_channel, (Base.catch_stack(), true))
+            else
+                put!(Base.active_repl_backend.response_channel, (err, catch_backtrace()))
+            end
         end
     end
-    return true
 end

src/precompile.jl

@@ -3,8 +3,7 @@ function _precompile_()
 
     @assert precompile(Tuple{typeof(watch_manifest), String})
     @assert precompile(Tuple{typeof(watch_file), String, Int})
-    @assert precompile(Tuple{Rescheduler{typeof(watch_manifest), String}})
-    @assert precompile(Tuple{Rescheduler{typeof(revise_dir_queued),Tuple{String}}})
+    @assert precompile(Tuple{TaskThunk})
     @assert precompile(Tuple{typeof(revise)})
     @assert precompile(Tuple{typeof(includet), String})
     # setindex! doesn't fully precompile, but it's still beneficial to do it

src/types.jl

@@ -216,37 +216,14 @@ function Base.showerror(io::IO, ex::GitRepoException)
 end
 
 """
-    Rescheduler(f, args)
+    thunk = TaskThunk(f, args)
 
-To facilitate precompilation and reduce latency, we replace
-
-```julia
-function watch_manifest(mfile)
-    wait_changed(mfile)
-    # stuff
-    @async watch_manifest(mfile)
-end
-
-@async watch_manifest(mfile)
-```
-
-with a rescheduling type:
-
-```julia
-fresched = Rescheduler(watch_manifest, (mfile,))
-schedule(Task(fresched))
-```
-
-where now `watch_manifest(mfile)` should return `true` if the task
-should be rescheduled after completion, and `false` otherwise.
+To facilitate precompilation and reduce latency, we avoid creation of anonymous thunks.
+`thunk` can be used as an argument in `schedule(Task(thunk))`.
 """
-struct Rescheduler{F,A}
-    f::F
-    args::A
+struct TaskThunk
+    f          # deliberately untyped
+    args       # deliberately untyped
 end
 
-function (thunk::Rescheduler{F,A})() where {F,A}
-    if thunk.f(thunk.args...)::Bool
-        schedule(Task(thunk))
-    end
-end
+@noinline (thunk::TaskThunk)() = thunk.f(thunk.args...)

test/runtests.jl

@@ -2616,7 +2616,7 @@ do_test("Switching free/dev") && @testset "Switching free/dev" begin
     devpath = joinpath(depot, "dev")
     mkpath(devpath)
     mfile = Revise.manifest_file()
-    schedule(Task(Revise.Rescheduler(Revise.watch_manifest, (mfile,))))
+    schedule(Task(Revise.TaskThunk(Revise.watch_manifest, (mfile,))))
     sleep(mtimedelay)
     pkgdevpath = make_a2d(devpath, 2, "w"; generate=false)
     cp(joinpath(ropkgpath, "Project.toml"), joinpath(devpath, "A2D/Project.toml"))