Skip to content

Commit 0cfeb00

Browse files
fix: Optionally exclude directories from list_files_under for performance (#1488)
This PR adds an `exclude_directories` argument to `repository_files.list_files_under` which resulted in a massive performance improvement for packages with lots of resources. I noticed that it was taking a really long time to "fetch" a package that was local to my machine, so there was no actual network operations required. I dug into the issue and it's actually from a change I added in [this PR](#1039) to expand certain types of resources into a list of their assets. The overwhelming majority of the time was being spent on the `if not repository_files.is_directory(repository_ctx, p)` check inside of the loop. By omitting directories from the result of `list_files_under` we can skip all of that work. ### Before - 225s <img width="1543" alt="image" src="https://github.com/user-attachments/assets/67f6f5e2-66b7-4143-95fd-8401ea6c8347" /> ### After - 2s <img width="1300" alt="image" src="https://github.com/user-attachments/assets/396c6eb2-4b59-41ab-811d-3e10bacba243" /> I didn't see any uses of `list_files_under` that were relying on it to contain directories. Should we change the behavior to never include directories? Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent 881ec3f commit 0cfeb00

File tree

2 files changed

+12
-20
lines changed

2 files changed

+12
-20
lines changed

swiftpkg/internal/pkginfos.bzl

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -309,15 +309,12 @@ def _new_target_from_json_maps(
309309
for directory in resource_directories:
310310
sets.remove(resources_set, directory)
311311

312-
resource_files = [
313-
p
314-
for p in repository_files.list_files_under(
315-
repository_ctx,
316-
directory.path,
317-
exclude_paths = exclude_paths,
318-
)
319-
if not repository_files.is_directory(repository_ctx, p)
320-
]
312+
resource_files = repository_files.list_files_under(
313+
repository_ctx,
314+
directory.path,
315+
exclude_paths = exclude_paths,
316+
exclude_directories = True,
317+
)
321318

322319
for p in resource_files:
323320
res = _new_resource_from_discovered_resource(p)
@@ -1023,23 +1020,14 @@ def _new_swift_src_info_from_sources(
10231020
repository_ctx,
10241021
target_path,
10251022
exclude_paths = exclude_paths,
1023+
exclude_directories = True,
10261024
)
10271025

1028-
# Identify the directories
1029-
directories = repository_files.list_directories_under(
1030-
repository_ctx,
1031-
target_path,
1032-
exclude_paths = exclude_paths,
1033-
)
1034-
dirs_set = sets.make(directories)
1035-
10361026
# The paths should be relative to the target not the root of the workspace.
1037-
# Do not include directories in the output.
10381027
discovered_res_files = [
10391028
f
10401029
for f in all_target_files
1041-
if not sets.contains(dirs_set, f) and
1042-
resource_files.is_auto_discovered_resource(f)
1030+
if resource_files.is_auto_discovered_resource(f)
10431031
]
10441032

10451033
return _new_swift_src_info(

swiftpkg/internal/repository_files.bzl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ def _list_files_under(
6565
repository_ctx,
6666
path,
6767
exclude_paths = [],
68+
exclude_directories = False,
6869
by_name = None,
6970
depth = None):
7071
"""Retrieves the list of files under the specified path.
@@ -76,6 +77,7 @@ def _list_files_under(
7677
path: A path `string` value.
7778
exclude_paths: Optional. A `list` of path `string` values that should be
7879
excluded from the result.
80+
exclude_directories: Optional. Exclude directories from the result.
7981
by_name: Optional. The name pattern that must be matched.
8082
depth: Optional. The depth as an `int` at which the directory must
8183
live from the starting path.
@@ -95,6 +97,8 @@ def _list_files_under(
9597
find_args.extend(["-mindepth", depth_str, "-maxdepth", depth_str])
9698
if by_name != None:
9799
find_args.extend(["-name", by_name])
100+
if exclude_directories:
101+
find_args.extend(["-not", "-type", "d"])
98102
exec_result = repository_ctx.execute(find_args, quiet = True)
99103
if exec_result.return_code != 0:
100104
fail("Failed to list files in %s. stderr:\n%s" % (path, exec_result.stderr))

0 commit comments

Comments
 (0)