copy_to_directory performance improvements (#236)

* reduce use of depset.to_list()

* avoid copying arrays when invoking copy_file

* fixup! reduce use of depset.to_list()

* fixup! reduce use of depset.to_list()
This commit is contained in:
Jason Bedard 2022-09-12 18:01:25 -07:00 committed by GitHub
parent 47be61d96b
commit 803dbf815e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 36 additions and 45 deletions

View File

@ -62,7 +62,7 @@ other rule implementations where additional_files can also be passed in.
| <a id="copy_to_directory_action-ctx"></a>ctx | The rule context. | none |
| <a id="copy_to_directory_action-srcs"></a>srcs | Files and/or directories or targets that provide DirectoryPathInfo to copy into the output directory. | none |
| <a id="copy_to_directory_action-dst"></a>dst | The directory to copy to. Must be a TreeArtifact. | none |
| <a id="copy_to_directory_action-additional_files"></a>additional_files | Additional files to copy that are not in the DefaultInfo or DirectoryPathInfo of srcs | <code>[]</code> |
| <a id="copy_to_directory_action-additional_files"></a>additional_files | List or depset of additional files to copy that are not in the DefaultInfo or DirectoryPathInfo of srcs | <code>[]</code> |
| <a id="copy_to_directory_action-root_paths"></a>root_paths | List of paths that are roots in the output directory.<br><br>See copy_to_directory rule documentation for more details. | <code>["."]</code> |
| <a id="copy_to_directory_action-include_external_repositories"></a>include_external_repositories | List of external repository names to include in the output directory.<br><br>See copy_to_directory rule documentation for more details. | <code>[]</code> |
| <a id="copy_to_directory_action-include_srcs_packages"></a>include_srcs_packages | List of Bazel packages to include in output directory.<br><br>See copy_to_directory rule documentation for more details. | <code>["**"]</code> |

View File

@ -653,7 +653,7 @@ def copy_to_directory_action(
dst: The directory to copy to. Must be a TreeArtifact.
additional_files: Additional files to copy that are not in the DefaultInfo or DirectoryPathInfo of srcs
additional_files: List or depset of additional files to copy that are not in the DefaultInfo or DirectoryPathInfo of srcs
root_paths: List of paths that are roots in the output directory.
@ -717,45 +717,33 @@ def copy_to_directory_action(
# Gather a list of src_path, dst_path pairs
found_input_paths = False
src_dirs = []
src_depsets = []
copy_paths = []
for src in srcs:
if DirectoryPathInfo in src:
found_input_paths = True
src_path, output_path, src_file = _copy_paths(
src = src,
root_paths = root_paths,
include_external_repositories = include_external_repositories,
include_srcs_packages = include_srcs_packages,
exclude_srcs_packages = exclude_srcs_packages,
include_srcs_patterns = include_srcs_patterns,
exclude_srcs_patterns = exclude_srcs_patterns,
replace_prefixes = replace_prefixes,
)
if src_path != None:
dst_path = skylib_paths.normalize("/".join([dst.path, output_path]))
if not _merge_into_copy_path(copy_paths, src_path, dst_path, src_file):
copy_paths.append((src_path, dst_path, src_file))
src_dirs.append(src)
if DefaultInfo in src:
for src_file in src[DefaultInfo].files.to_list():
found_input_paths = True
src_path, output_path, src_file = _copy_paths(
src = src_file,
root_paths = root_paths,
include_external_repositories = include_external_repositories,
include_srcs_packages = include_srcs_packages,
exclude_srcs_packages = exclude_srcs_packages,
include_srcs_patterns = include_srcs_patterns,
exclude_srcs_patterns = exclude_srcs_patterns,
replace_prefixes = replace_prefixes,
)
if src_path != None:
dst_path = skylib_paths.normalize("/".join([dst.path, output_path]))
if not _merge_into_copy_path(copy_paths, src_path, dst_path, src_file):
copy_paths.append((src_path, dst_path, src_file))
for additional_file in additional_files:
src_depsets.append(src[DefaultInfo].files)
# Convert potentially-large arrays into slices to pass by reference
# instead of copying when invoking _copy_paths()
root_paths = root_paths[:]
include_external_repositories = include_external_repositories[:]
include_srcs_packages = include_srcs_packages[:]
exclude_srcs_packages = exclude_srcs_packages[:]
include_srcs_patterns = include_srcs_patterns[:]
exclude_srcs_patterns = exclude_srcs_patterns[:]
if type(additional_files) == "list":
additional_files = depset(additional_files)
all_srcs = src_dirs + depset(transitive = [additional_files] + src_depsets).to_list()
for src in all_srcs:
found_input_paths = True
src_path, output_path, src_file = _copy_paths(
src = additional_file,
src = src,
root_paths = root_paths,
include_external_repositories = include_external_repositories,
include_srcs_packages = include_srcs_packages,

View File

@ -37,6 +37,8 @@ def _split_on(expr, splits):
result.append(accumulator)
return result
GLOB_SYMBOLS = ["**", "*", "?"]
def glob_match(expr, path, match_path_separator = False):
"""Test if the passed path matches the glob expression.
@ -61,7 +63,7 @@ def glob_match(expr, path, match_path_separator = False):
if expr.find("***") != -1:
fail("glob_match: invalid *** pattern found in glob expression")
expr_parts = _split_on(expr, ["**", "*", "?"])
expr_parts = _split_on(expr, GLOB_SYMBOLS[:])
for i, expr_part in enumerate(expr_parts):
if expr_part == "**":

View File

@ -16,8 +16,11 @@ def _output_files(ctx):
files_depset = ctx.attr.target[OutputGroupInfo][ctx.attr.output_group]
else:
files_depset = ctx.attr.target[DefaultInfo].files
files_list = files_depset.to_list()
for path in ctx.attr.paths:
file = _find_short_path_in_files_depset(files_depset, path)
file = _find_short_path_in_files_list(files_list, path)
if not file:
if ctx.attr.output_group:
msg = "%s file not found within the %s output group of %s" % (path, ctx.attr.output_group, ctx.attr.target)
@ -70,18 +73,16 @@ def make_output_files(name, target, paths, **kwargs):
)
return _to_label(name)
def _find_short_path_in_files_depset(files_depset, short_path):
def _find_short_path_in_files_list(files_list, short_path):
"""Helper function find a file in a DefaultInfo by short path
Args:
files_depset: a depset
files_list: a list of files
short_path: the short path (path relative to root) to search for
Returns:
The File if found else None
"""
if files_depset:
for file in files_depset.to_list():
if file.short_path == short_path:
return file
return None
for file in files_list:
if file.short_path == short_path:
return file
return None