REPLCompletions: PATH caching tweaks (JuliaLang#52893)

stdlib/REPL/src/REPLCompletions.jl

@@ -271,25 +271,34 @@ end
 
 const PATH_cache_lock = Base.ReentrantLock()
 const PATH_cache = Set{String}()
-cached_PATH_string::Union{String,Nothing} = nothing
-function cached_PATH_changed()
- global cached_PATH_string
- @lock(PATH_cache_lock, cached_PATH_string) !== get(ENV, "PATH", nothing)
+PATH_cache_task::Union{Task,Nothing} = nothing # used for sync in tests
+next_cache_update::Float64 = 0.0
+function maybe_spawn_cache_PATH()
+ global PATH_cache_task, next_cache_update
+ @lock PATH_cache_lock begin
+ PATH_cache_task isa Task && !istaskdone(PATH_cache_task) && return
+ time() < next_cache_update && return
+ PATH_cache_task = Threads.@spawn REPLCompletions.cache_PATH()
+ Base.errormonitor(PATH_cache_task)
+ end
 end
-const PATH_cache_finished = Base.Condition() # used for sync in tests
 
 # caches all reachable files in PATH dirs
 function cache_PATH()
- global cached_PATH_string
- path = @lock PATH_cache_lock begin
- empty!(PATH_cache)
- cached_PATH_string = get(ENV, "PATH", nothing)
- end
+ path = get(ENV, "PATH", nothing)
  path isa String || return
 
+ global next_cache_update
+
+ # Calling empty! on PATH_cache would be annoying for async typing hints as completions would temporarily disappear.
+ # So keep track of what's added this time and at the end remove any that didn't appear this time from the global cache.
+ this_PATH_cache = Set{String}()
+
  @debug "caching PATH files" PATH=path
  pathdirs = split(path, @static Sys.iswindows() ? ";" : ":")
 
+ next_yield_time = time() + 0.01
+
  t = @elapsed for pathdir in pathdirs
  actualpath = try
  realpath(pathdir)
@@ -322,6 +331,7 @@ function cache_PATH()
  try
  if isfile(joinpath(pathdir, file))
  @lock PATH_cache_lock push!(PATH_cache, file)
+ push!(this_PATH_cache, file)
  end
  catch e
  # `isfile()` can throw in rare cases such as when probing a
@@ -333,10 +343,18 @@ function cache_PATH()
  rethrow()
  end
  end
- yield() # so startup doesn't block when -t1
+ if time() >= next_yield_time
+ yield() # to avoid blocking typing when -t1
+ next_yield_time = time() + 0.01
+ end
  end
  end
- notify(PATH_cache_finished)
+
+ @lock PATH_cache_lock begin
+ intersect!(PATH_cache, this_PATH_cache) # remove entries from PATH_cache that weren't found this time
+ next_cache_update = time() + 10 # earliest next update can run is 10s after
+ end
+
  @debug "caching PATH files took $t seconds" length(pathdirs) length(PATH_cache)
  return PATH_cache
 end
@@ -380,15 +398,13 @@ function complete_path(path::AbstractString;
  end
 
  if use_envpath && isempty(dir)
- # Look for files in PATH as well. These are cached in `cache_PATH` in a separate task in REPL init.
- # If we cannot get lock because its still caching just pass over this so that initial
- # typing isn't laggy. If the PATH string has changed since last cache re-cache it
- cached_PATH_changed() && Base.errormonitor(Threads.@spawn REPLCompletions.cache_PATH())
- if trylock(PATH_cache_lock)
+ # Look for files in PATH as well. These are cached in `cache_PATH` in an async task to not block typing.
+ # If we cannot get lock because its still caching just pass over this so that typing isn't laggy.
+ maybe_spawn_cache_PATH() # only spawns if enough time has passed and the previous caching task has completed
+ @lock PATH_cache_lock begin
  for file in PATH_cache
  startswith(file, prefix) && push!(matches, file)
  end
- unlock(PATH_cache_lock)
  end
  end
 

stdlib/REPL/test/replcompletions.jl

@@ -1146,11 +1146,15 @@ let s, c, r
  # PATH can also contain folders which we aren't actually allowed to read.
  withenv("PATH" => string(path, ":", unreadable)) do
  s = "tmp-execu"
- c,r = test_scomplete(s)
  # Files reachable by PATH are cached async when PATH is seen to have been changed by `complete_path`
  # so changes are unlikely to appear in the first complete. For testing purposes we can wait for
  # caching to finish
- wait(REPL.REPLCompletions.PATH_cache_finished)
+ @lock REPL.REPLCompletions.PATH_cache_lock begin
+ # force the next cache update to happen immediately
+ REPL.REPLCompletions.next_cache_update = 0
+ end
+ c,r = test_scomplete(s)
+ wait(REPL.REPLCompletions.PATH_cache_task::Task) # wait for caching to complete
  c,r = test_scomplete(s)
  @test "tmp-executable" in c
  @test r == 1:9
@@ -1179,8 +1183,12 @@ let s, c, r
 
  withenv("PATH" => string(tempdir(), ":", dir)) do
  s = string("repl-completio")
+ @lock REPL.REPLCompletions.PATH_cache_lock begin
+ # force the next cache update to happen immediately
+ REPL.REPLCompletions.next_cache_update = 0
+ end
  c,r = test_scomplete(s)
- wait(REPL.REPLCompletions.PATH_cache_finished) # wait for caching to complete
+ wait(REPL.REPLCompletions.PATH_cache_task::Task) # wait for caching to complete
  c,r = test_scomplete(s)
  @test ["repl-completion"] == c
  @test s[r] == "repl-completio"