From 636bc197f74e0aea0efb0221049aebb6597f4b0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Drvo=C5=A1t=C4=9Bp?= Date: Tue, 16 Sep 2025 12:28:07 +0200 Subject: [PATCH 1/5] Don't spawn a task per file to include, use a pool of tasks instead --- src/ReTestItems.jl | 132 ++++++++++++++++++++++++++++++--------------- 1 file changed, 88 insertions(+), 44 deletions(-) diff --git a/src/ReTestItems.jl b/src/ReTestItems.jl index 99674a7..f65ea01 100644 --- a/src/ReTestItems.jl +++ b/src/ReTestItems.jl @@ -802,7 +802,84 @@ function _is_subproject(dir, current_projectfile) return true end -# for each directory, kick off a recursive test-finding task +const _hidden_re = r"\.\w" + +# Traverses the directory tree starting at `project_root` and populates `dir_nodes` with +# `DirNode`s and `FileNode`s for each directory and test file found. Filters out non-eligible +# paths. +function walkdir_task(walkdir_channel::Channel{Tuple{String,FileNode}}, project_root::String, root_node, dir_nodes, subproject_root::Union{String,Nothing}, ti_filter, paths, projectfile, report, verbose_results) + try + # Since test items don't store paths to their test setups, we need to traverse the + # whole project, not just the requested paths. + stack = [project_root] + while !isempty(stack) + root = pop!(stack) + rel_root = nestedrelpath(root, project_root) + dir_node = DirNode(rel_root; report, verbose=verbose_results) + dir_nodes[rel_root] = dir_node + push!(get(dir_nodes, dirname(rel_root), root_node), dir_node) + for file in readdir(root) + startswith(file, _hidden_re) && continue # skip hidden files/directories + full_path = joinpath(root, file) + if isdir(full_path) + if subproject_root !== nothing && startswith(full_path, subproject_root) + @debugv 1 "Skipping files in `$root` in subproject `$subproject_root`" + continue + elseif _is_subproject(root, projectfile) + subproject_root = root + continue + end + push!(stack, full_path) + else + # We filter here, rather than the testitem level, to make sure we don't + # `include` a file that isn't supposed to be a test-file at all, e.g. its + # not on a path the user requested but it happens to have a test-file suffix. + # We always include testsetup-files so users don't need to request them, + # even if they're not in a requested path, e.g. they are a level up in the + # directory tree. The testsetup-file suffix is hopefully specific enough + # to ReTestItems that this doesn't lead to `include`ing unexpected files. + if !(is_testsetup_file(full_path) || (is_test_file(full_path) && is_requested(full_path, paths))) + continue + end + rel_full_path = nestedrelpath(full_path, project_root) + file_node = FileNode(rel_full_path, ti_filter; report, verbose=verbose_results) + push!(dir_node, file_node) + put!(walkdir_channel, (full_path, file_node)) + end + end + end + close(walkdir_channel) + catch err + close(walkdir_channel, err) + rethrow(err) + end + return nothing +end + +# Parses and evals files found by the `walkdir_task`. During macro expansion of `@testitem` +# test items are push!d onto the FileNode stored in task local storage as `:__RE_TEST_ITEMS__`. +function include_task(walkdir_channel, setup_channel, project_root, ti_filter) + try + testitem_names = Set{String}() # to enforce that names in the same file are unique + task_local_storage(:__RE_TEST_RUNNING__, true) do + task_local_storage(:__RE_TEST_PROJECT__, project_root) do + task_local_storage(:__RE_TEST_SETUPS__, setup_channel) do + for (file_path, file_node) in walkdir_channel + @debugv 1 "Including test items from file `$(file_path)`" + task_local_storage(:__RE_TEST_ITEMS__, (file_node, empty!(testitem_names))) do + Base.include(ti_filter, Main, file_path) + end + end + end + end + end + catch err + close(walkdir_channel, err) + rethrow(err) + end +end + +# Find test items using a pool of tasks to include files in parallel. # Returns (testitems::TestItems, setups::Dict{Symbol,TestSetup}) # Assumes `isdir(project_root)`, which is guaranteed by `_runtests`. function include_testfiles!(project_name, projectfile, paths, ti_filter::TestItemFilter, verbose_results::Bool, report::Bool) @@ -823,51 +900,18 @@ function include_testfiles!(project_name, projectfile, paths, ti_filter::TestIte end return setups end - hidden_re = r"\.\w" - @sync for (root, d, files) in Base.walkdir(project_root) - if subproject_root !== nothing && startswith(root, subproject_root) - @debugv 1 "Skipping files in `$root` in subproject `$subproject_root`" - continue - elseif _is_subproject(root, projectfile) - subproject_root = root - continue - end - rpath = nestedrelpath(root, project_root) - startswith(rpath, hidden_re) && continue # skip hidden directories - dir_node = DirNode(rpath; report, verbose=verbose_results) - dir_nodes[rpath] = dir_node - push!(get(dir_nodes, dirname(rpath), root_node), dir_node) - for file in files - startswith(file, hidden_re) && continue # skip hidden files - filepath = joinpath(root, file) - # We filter here, rather than the testitem level, to make sure we don't - # `include` a file that isn't supposed to be a test-file at all, e.g. its - # not on a path the user requested but it happens to have a test-file suffix. - # We always include testsetup-files so users don't need to request them, - # even if they're not in a requested path, e.g. they are a level up in the - # directory tree. The testsetup-file suffix is hopefully specific enough - # to ReTestItems that this doesn't lead to `include`ing unexpected files. - if !(is_testsetup_file(filepath) || (is_test_file(filepath) && is_requested(filepath, paths))) - continue - end - fpath = nestedrelpath(filepath, project_root) - file_node = FileNode(fpath, ti_filter; report, verbose=verbose_results) - testitem_names = Set{String}() # to enforce that names in the same file are unique - push!(dir_node, file_node) - @debugv 1 "Including test items from file `$filepath`" - @spawn begin - task_local_storage(:__RE_TEST_RUNNING__, true) do - task_local_storage(:__RE_TEST_ITEMS__, ($file_node, $testitem_names)) do - task_local_storage(:__RE_TEST_PROJECT__, $(project_root)) do - task_local_storage(:__RE_TEST_SETUPS__, $setup_channel) do - Base.include($ti_filter, Main, $filepath) - end - end - end - end - end + + walkdir_channel = Channel{Tuple{String, FileNode}}(1024) + @sync begin + @spawn walkdir_task( + $walkdir_channel, $project_root, $root_node, $dir_nodes, $subproject_root, + $ti_filter, $paths, $projectfile, $report, $verbose_results + ) + for _ in 1:clamp(2*nthreads(), 1, 16) + @spawn include_task($walkdir_channel, $setup_channel, $project_root, $ti_filter) end end + @debugv 2 "Finished including files" # finished including all test files, so finalize our graph # prune empty directories/files From 9da139d10a4ccfb9a7802bb263cb84f5cb2f31e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Drvo=C5=A1t=C4=9Bp?= Date: Tue, 16 Sep 2025 12:45:20 +0200 Subject: [PATCH 2/5] . --- src/ReTestItems.jl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ReTestItems.jl b/src/ReTestItems.jl index f65ea01..e081bae 100644 --- a/src/ReTestItems.jl +++ b/src/ReTestItems.jl @@ -804,10 +804,12 @@ end const _hidden_re = r"\.\w" -# Traverses the directory tree starting at `project_root` and populates `dir_nodes` with +# Traverses the directory tree starting at `project_root` and grows `root_node` with # `DirNode`s and `FileNode`s for each directory and test file found. Filters out non-eligible # paths. -function walkdir_task(walkdir_channel::Channel{Tuple{String,FileNode}}, project_root::String, root_node, dir_nodes, subproject_root::Union{String,Nothing}, ti_filter, paths, projectfile, report, verbose_results) +function walkdir_task(walkdir_channel::Channel{Tuple{String,FileNode}}, project_root::String, root_node, ti_filter, paths, projectfile, report, verbose_results) + dir_nodes = Dict{String, DirNode}() + subproject_root = nothing # don't recurse into directories with their own Project.toml. try # Since test items don't store paths to their test setups, we need to traverse the # whole project, not just the requested paths. @@ -877,6 +879,7 @@ function include_task(walkdir_channel, setup_channel, project_root, ti_filter) close(walkdir_channel, err) rethrow(err) end + return nothing end # Find test items using a pool of tasks to include files in parallel. @@ -884,9 +887,7 @@ end # Assumes `isdir(project_root)`, which is guaranteed by `_runtests`. function include_testfiles!(project_name, projectfile, paths, ti_filter::TestItemFilter, verbose_results::Bool, report::Bool) project_root = dirname(projectfile) - subproject_root = nothing # don't recurse into directories with their own Project.toml. root_node = DirNode(project_name; report, verbose=verbose_results) - dir_nodes = Dict{String, DirNode}() # setup_channel is populated in store_test_setup when we expand a @testsetup # we set it below in tls as __RE_TEST_SETUPS__ for each included file setup_channel = Channel{Pair{Symbol, TestSetup}}(Inf) @@ -904,8 +905,7 @@ function include_testfiles!(project_name, projectfile, paths, ti_filter::TestIte walkdir_channel = Channel{Tuple{String, FileNode}}(1024) @sync begin @spawn walkdir_task( - $walkdir_channel, $project_root, $root_node, $dir_nodes, $subproject_root, - $ti_filter, $paths, $projectfile, $report, $verbose_results + $walkdir_channel, $project_root, $root_node, $ti_filter, $paths, $projectfile, $report, $verbose_results ) for _ in 1:clamp(2*nthreads(), 1, 16) @spawn include_task($walkdir_channel, $setup_channel, $project_root, $ti_filter) From bcbb1db3fa1bb7ad145129568b973b8ba1ecba0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Drvo=C5=A1t=C4=9Bp?= Date: Sat, 20 Sep 2025 15:23:15 +0200 Subject: [PATCH 3/5] . --- src/ReTestItems.jl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ReTestItems.jl b/src/ReTestItems.jl index e081bae..1470f64 100644 --- a/src/ReTestItems.jl +++ b/src/ReTestItems.jl @@ -802,7 +802,8 @@ function _is_subproject(dir, current_projectfile) return true end -const _hidden_re = r"\.\w" +# called on results of `readdir(root)` +_is_hidden(name::AbstractString) = ncodeunits(name) > 1 && name[1] == '.' # Traverses the directory tree starting at `project_root` and grows `root_node` with # `DirNode`s and `FileNode`s for each directory and test file found. Filters out non-eligible @@ -821,7 +822,7 @@ function walkdir_task(walkdir_channel::Channel{Tuple{String,FileNode}}, project_ dir_nodes[rel_root] = dir_node push!(get(dir_nodes, dirname(rel_root), root_node), dir_node) for file in readdir(root) - startswith(file, _hidden_re) && continue # skip hidden files/directories + _is_hidden(file) && continue # skip hidden files/directories full_path = joinpath(root, file) if isdir(full_path) if subproject_root !== nothing && startswith(full_path, subproject_root) @@ -907,7 +908,7 @@ function include_testfiles!(project_name, projectfile, paths, ti_filter::TestIte @spawn walkdir_task( $walkdir_channel, $project_root, $root_node, $ti_filter, $paths, $projectfile, $report, $verbose_results ) - for _ in 1:clamp(2*nthreads(), 1, 16) + for _ in 1:clamp(2*(nthreads()-(nthreads() == 1)), 1, 16) # 1 to 16 tasks, 1 if single-threaded @spawn include_task($walkdir_channel, $setup_channel, $project_root, $ti_filter) end end From d2f8a3de5eb80e04309ae7d8b73a421dbbd28013 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Drvo=C5=A1t=C4=9Bp?= Date: Sun, 21 Sep 2025 00:18:09 +0200 Subject: [PATCH 4/5] Fewer allocs --- src/ReTestItems.jl | 52 +++++++++++++++++++++++++--------------------- test/internals.jl | 6 +++--- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/ReTestItems.jl b/src/ReTestItems.jl index 1470f64..ecb0554 100644 --- a/src/ReTestItems.jl +++ b/src/ReTestItems.jl @@ -789,49 +789,53 @@ function nestedrelpath(path::T, startdir::AbstractString) where {T <: AbstractSt end # is `dir` the root of a subproject inside the current project? -function _is_subproject(dir, current_projectfile) - projectfile = _project_file(dir) - isnothing(projectfile) && return false - - projectfile = abspath(projectfile) - projectfile == current_projectfile && return false - # a `test/Project.toml` is special and doesn't indicate a subproject - current_project_dir = dirname(current_projectfile) - rel_projectfile = nestedrelpath(projectfile, current_project_dir) - rel_projectfile == joinpath("test", "Project.toml") && return false - return true +# all three paths are assumed to be absolute paths +let test_project = joinpath("test", "Project.toml") + global function _is_subproject(dir, current_projectfile, current_project_dir) + projectfile = _project_file(dir) + isnothing(projectfile) && return false + + projectfile == current_projectfile && return false + # a `test/Project.toml` is special and doesn't indicate a subproject + rel_projectfile = nestedrelpath(projectfile, current_project_dir) + rel_projectfile == test_project && return false + return true + end end # called on results of `readdir(root)` -_is_hidden(name::AbstractString) = ncodeunits(name) > 1 && name[1] == '.' +_is_hidden(name::AbstractString) = ncodeunits(name) > 1 && codeunits(name)[1] == UInt8('.') # Traverses the directory tree starting at `project_root` and grows `root_node` with # `DirNode`s and `FileNode`s for each directory and test file found. Filters out non-eligible # paths. function walkdir_task(walkdir_channel::Channel{Tuple{String,FileNode}}, project_root::String, root_node, ti_filter, paths, projectfile, report, verbose_results) + @assert isabspath(project_root) + @assert isabspath(projectfile) dir_nodes = Dict{String, DirNode}() subproject_root = nothing # don't recurse into directories with their own Project.toml. + abspaths = map(abspath, paths) try # Since test items don't store paths to their test setups, we need to traverse the # whole project, not just the requested paths. stack = [project_root] while !isempty(stack) root = pop!(stack) + if subproject_root !== nothing && startswith(root, subproject_root) + @debugv 1 "Skipping files in `$root` in subproject `$subproject_root`" + continue + elseif _is_subproject(root, projectfile, project_root) + subproject_root = root + continue + end rel_root = nestedrelpath(root, project_root) dir_node = DirNode(rel_root; report, verbose=verbose_results) dir_nodes[rel_root] = dir_node push!(get(dir_nodes, dirname(rel_root), root_node), dir_node) - for file in readdir(root) + for file in readdir(root) # TODO: Use https://github.com/JuliaLang/julia/pull/55358 once it lands _is_hidden(file) && continue # skip hidden files/directories full_path = joinpath(root, file) if isdir(full_path) - if subproject_root !== nothing && startswith(full_path, subproject_root) - @debugv 1 "Skipping files in `$root` in subproject `$subproject_root`" - continue - elseif _is_subproject(root, projectfile) - subproject_root = root - continue - end push!(stack, full_path) else # We filter here, rather than the testitem level, to make sure we don't @@ -841,7 +845,7 @@ function walkdir_task(walkdir_channel::Channel{Tuple{String,FileNode}}, project_ # even if they're not in a requested path, e.g. they are a level up in the # directory tree. The testsetup-file suffix is hopefully specific enough # to ReTestItems that this doesn't lead to `include`ing unexpected files. - if !(is_testsetup_file(full_path) || (is_test_file(full_path) && is_requested(full_path, paths))) + if !(is_testsetup_file(full_path) || (is_test_file(full_path) && is_requested(full_path, abspaths))) continue end rel_full_path = nestedrelpath(full_path, project_root) @@ -962,9 +966,9 @@ end # Is filepath one of the paths the user requested? is_requested(filepath, paths::Tuple{}) = true # no paths means no restrictions -function is_requested(filepath, paths::Tuple) - return any(paths) do p - startswith(filepath, abspath(p)) +function is_requested(filepath, abspaths::Tuple) + return any(abspaths) do p + startswith(filepath, p) end end diff --git a/test/internals.jl b/test/internals.jl index aea8e56..1b215c9 100644 --- a/test/internals.jl +++ b/test/internals.jl @@ -75,11 +75,11 @@ end @assert isfile(monorepo_proj) for pkg in ("B", "C", "D") path = joinpath(monorepo, "monorepo_packages", pkg) - @test _is_subproject(path, monorepo_proj) + @test _is_subproject(path, monorepo_proj, monorepo) end for dir in ("src", "test") path = joinpath(monorepo, dir) - @test !_is_subproject(path, monorepo_proj) + @test !_is_subproject(path, monorepo_proj, monorepo) end # Test "test/Project.toml" does cause "test/" to be subproject tpf = joinpath(test_pkg_dir, "TestProjectFile.jl") @@ -88,7 +88,7 @@ end @assert isfile(joinpath(tpf, "test", "Project.toml")) for dir in ("src", "test") path = joinpath(tpf, dir) - @test !_is_subproject(path, tpf_proj) + @test !_is_subproject(path, tpf_proj, tpf) end end From 5ce27620333341a463082aa0e17c19168be3111a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Drvo=C5=A1t=C4=9Bp?= Date: Sun, 28 Sep 2025 09:31:36 +0200 Subject: [PATCH 5/5] Simplify _is_subproject --- src/ReTestItems.jl | 19 ++++++++----------- test/internals.jl | 6 +++--- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/ReTestItems.jl b/src/ReTestItems.jl index ecb0554..a00c197 100644 --- a/src/ReTestItems.jl +++ b/src/ReTestItems.jl @@ -789,16 +789,18 @@ function nestedrelpath(path::T, startdir::AbstractString) where {T <: AbstractSt end # is `dir` the root of a subproject inside the current project? -# all three paths are assumed to be absolute paths +# Both paths are assumed to be absolute paths let test_project = joinpath("test", "Project.toml") - global function _is_subproject(dir, current_projectfile, current_project_dir) + global function _is_subproject(dir, current_project_dir) projectfile = _project_file(dir) isnothing(projectfile) && return false - projectfile == current_projectfile && return false + dir == current_project_dir && return false + # a `test/Project.toml` is special and doesn't indicate a subproject rel_projectfile = nestedrelpath(projectfile, current_project_dir) rel_projectfile == test_project && return false + @debugv 1 "Skipping files under subproject `$projectfile`" return true end end @@ -812,8 +814,8 @@ _is_hidden(name::AbstractString) = ncodeunits(name) > 1 && codeunits(name)[1] == function walkdir_task(walkdir_channel::Channel{Tuple{String,FileNode}}, project_root::String, root_node, ti_filter, paths, projectfile, report, verbose_results) @assert isabspath(project_root) @assert isabspath(projectfile) + # The keys are `nestedrelpath`s which return `SubString{String}`s, but we used dirname below so we have to allocate a String :( dir_nodes = Dict{String, DirNode}() - subproject_root = nothing # don't recurse into directories with their own Project.toml. abspaths = map(abspath, paths) try # Since test items don't store paths to their test setups, we need to traverse the @@ -821,13 +823,8 @@ function walkdir_task(walkdir_channel::Channel{Tuple{String,FileNode}}, project_ stack = [project_root] while !isempty(stack) root = pop!(stack) - if subproject_root !== nothing && startswith(root, subproject_root) - @debugv 1 "Skipping files in `$root` in subproject `$subproject_root`" - continue - elseif _is_subproject(root, projectfile, project_root) - subproject_root = root - continue - end + # Don't recurse into directories with their own Project.toml. + _is_subproject(root, project_root) && continue rel_root = nestedrelpath(root, project_root) dir_node = DirNode(rel_root; report, verbose=verbose_results) dir_nodes[rel_root] = dir_node diff --git a/test/internals.jl b/test/internals.jl index 1b215c9..f855bc6 100644 --- a/test/internals.jl +++ b/test/internals.jl @@ -75,11 +75,11 @@ end @assert isfile(monorepo_proj) for pkg in ("B", "C", "D") path = joinpath(monorepo, "monorepo_packages", pkg) - @test _is_subproject(path, monorepo_proj, monorepo) + @test _is_subproject(path, monorepo) end for dir in ("src", "test") path = joinpath(monorepo, dir) - @test !_is_subproject(path, monorepo_proj, monorepo) + @test !_is_subproject(path, monorepo) end # Test "test/Project.toml" does cause "test/" to be subproject tpf = joinpath(test_pkg_dir, "TestProjectFile.jl") @@ -88,7 +88,7 @@ end @assert isfile(joinpath(tpf, "test", "Project.toml")) for dir in ("src", "test") path = joinpath(tpf, dir) - @test !_is_subproject(path, tpf_proj, tpf) + @test !_is_subproject(path, tpf) end end