Slow nixpkgs_package rule on macOS

[lamcw]

Describe the bug nixpkgs_package runs slow (~5 minutes) trying to copy all nix_file_deps to external repository root.

I noticed slow nixpkgs_package fetches, and upon investigation, I found out that

1. nixpkgs_package kept being restarted, possibly due to missing dependencies for the rule itself, or something related to https://github.com/bazelbuild/bazel/issues/10515

2. copying all nix_file_deps using cp takes most of the time in _nixpkgs_package_impl. See screenshot of bazel profile below -- each cp call takes 50ms to 60ms to run, multiply that by 4200 invocations it can delay the build by up to 5 minutes. time to run one single cp call

   number of cp invocations in a bazel build command

   And found out that by removing the following chmod call in the cp function improves the performance significantly (5mins -> 1min) so it seems like this is the bottleneck for the nixpkgs_package repository rule https://github.com/tweag/rules_nixpkgs/blob/9f08fb2322050991dead17c8d10d453650cf92b7/core/util.bzl#L41-L46 My guess is that syscalls (stats) on macOS is slow and therefore creates a bottleneck for the WORKSPACE analysis.

So wanted to raise the issue for awareness, and also this might be good information for others. Is there any workarounds to not use chmod while retaining file permissions when copying files?

To Reproduce In WORKSPACE, define the following

# big list of nix files that `nixpkgs.nix` depends on
# for me 70 files is enough to hold up the build for 5 minutes
nix_files = [...]

nixpkgs_package(
    name = "python3",
    attribute_path = "python39",
    build_file_content = """
package(default_visibility = ["//visibility:public"])
filegroup(
    name = "interpreter",
    srcs = ["bin/python"],
)
    """,
    nix_file = ":nixpkgs.nix",
    nix_file_deps = nix_files,
    repository = "@nixpkgs",
)

Run bazel build //target/that/depends/on/python3 and watch bazel getting stuck at "fetching" the python3 repository. 😢

Expected behavior bazel build should not spend long time w

Environment

• OS name + version: macOS Monterey version 12.5
• Version of the code: commit 210d30a81cedde04b4281fd163428722278fddfb

[aherrmann]

@lamcw Thank you for raising and debugging this and for the detailed report!

nixpkgs_package kept being restarted, possibly due to missing dependencies for the rule itself, or something related to https://github.com/bazelbuild/bazel/issues/10515

Perhaps related to that issue. The src comes from nix_file_deps, which is a label_list, so it shouldn't be that. But, it could be the path conversion on dep.

Is there any workarounds to not use chmod while retaining file permissions when copying files?

A bit of background on that line, it comes from the fact that the repository rule Starlark API offers no straight-forward way to copy a file (including its executable bit), see https://github.com/tweag/rules_nixpkgs/pull/161 and https://github.com/bazelbuild/bazel/issues/11858 for the Bazel feature request. Ideally, Bazel would simply offer repository_ctx.copy to that end. Perhaps a batch version of this loop that shells out to an external process to do all the copying could be formulated?

[lamcw]

Perhaps a batch version of this loop that shells out to an external process to do all the copying could be formulated?

Oh that could solve the problem. An alternative solution could also be something that looks like

def is_executable(repository_ctx, path):
    result = repository_ctx.execute(["stat", "-c", "%a", path])
    return any_executable_bit(result.stdout)

src_path = repository_ctx.path(src)
repository_ctx.file(
    repository_ctx.path(dest),
    repository_ctx.read(src_path),
    executable = is_executable(src_path),
    legacy_utf8 = False,
)

although I am not sure if stats are any faster than chmods...

[aherrmann]

although I am not sure if stats are any faster than chmods...

Good question, I don't have a Mac handy atm to benchmark. Also something to keep in mind with that approach: BSD stat takes different options from GNU stat.

[lamcw]

Aha I just experimented a bit with a function like this:

def _is_executable(repository_ctx, path):
    exec_result = repository_ctx.execute(["stat", "-c", "%a", path])
    stdout = exec_result.stdout.strip()
    mode = int(stdout, 8)
    return mode & 0o100 != 0

repository_ctx.file(
    repository_ctx.path(dest),
    repository_ctx.read(src_path),
    executable = _is_executable(src_path),
    legacy_utf8 = False,
)

and it seems like the performance much better -- it adds a tad bit of overhead (5-10s?) but it's still faster than a 5 minute build. I will upstream the changes back soon.

[benradf]

I started looking into doing the copying with an external process as @aherrmann suggested (something like tar -c -T files.list | tar -x -C $dest_dir) but I don't have a working solution yet.

Your approach seems less drastic @lamcw. If it improves the performance sufficiently I say we go with that. I am a little curious why chmod is slower than stat though.

[benradf]

I've created the above draft pull request with my tar solution but I'm not sure how much of an improvement it is since I don't have a Mac to test. On Linux it's marginally quicker but not enough to be worth merging I think.

[lamcw]

I am a little curious why chmod is slower than stat though.

Did a bit of research, and it looks like chmod (in our case, coreutils chmod) does more than just changing the mode. It also calls fstatat for every file it changed. Which probably explains why it is slower since stat only runs one fstat as opposed to "mode change" + fstatat in chmod.

[benradf]

Ah that makes sense, thanks for looking into it further.