Skip to content

Improve repinning performance by not recomputing cache path for every artifact #1108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 23, 2024
Merged

Improve repinning performance by not recomputing cache path for every artifact #1108

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b15419
Improve coursier_fetch performance by not recomputing cache path for …
BalestraPatrick Apr 23, 2024
37aab4e
Move comment to new location
BalestraPatrick Apr 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions coursier.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ def _normalize_to_unix_path(path):
# Relativize an absolute path to an artifact in coursier's default cache location.
# After relativizing, also symlink the path into the workspace's output base.
# Then return the relative path for further processing
def _relativize_and_symlink_file_in_coursier_cache(repository_ctx, absolute_path):
def _relativize_and_symlink_file_in_coursier_cache(repository_ctx, absolute_path, coursier_cache_path):
# The path manipulation from here on out assumes *nix paths, not Windows.
# for artifact_absolute_path in artifact_absolute_paths:
#
Expand All @@ -216,13 +216,6 @@ def _relativize_and_symlink_file_in_coursier_cache(repository_ctx, absolute_path
# Also, replace '//' with '/', otherwise parsing of the file path for the
# coursier cache will fail if variables like HOME or COURSIER_CACHE have a
# trailing slash.
#
# We assume that coursier uses the default cache location
# TODO(jin): allow custom cache locations
coursier_cache_path = get_coursier_cache_or_default(
repository_ctx,
repository_ctx.attr.name.startswith("unpinned_"),
).replace("//", "/")
absolute_path_parts = absolute_path.split(coursier_cache_path)
if len(absolute_path_parts) != 2:
fail("Error while trying to parse the path of file in the coursier cache: " + absolute_path)
Expand Down Expand Up @@ -1007,6 +1000,13 @@ def _coursier_fetch_impl(repository_ctx):

files_to_inspect = []

# We assume that coursier uses the default cache location
# TODO(jin): allow custom cache locations
coursier_cache_path = get_coursier_cache_or_default(
repository_ctx,
repository_ctx.attr.name.startswith("unpinned_"),
).replace("//", "/")

for artifact in dep_tree["dependencies"]:
# Some artifacts don't contain files; they are just parent artifacts
# to other artifacts.
Expand Down Expand Up @@ -1034,7 +1034,7 @@ def _coursier_fetch_impl(repository_ctx):
continue

if repository_ctx.attr.name.startswith("unpinned_"):
artifact.update({"file": _relativize_and_symlink_file_in_coursier_cache(repository_ctx, artifact["file"])})
artifact.update({"file": _relativize_and_symlink_file_in_coursier_cache(repository_ctx, artifact["file"], coursier_cache_path)})

# Coursier saves the artifacts into a subdirectory structure
# that mirrors the URL where the artifact's fetched from. Using
Expand Down