Skip to content

Commit

Permalink
perf: reduce size of npm_package_store transitive_files depset (#1594)
Browse files Browse the repository at this point in the history
  • Loading branch information
gregmagolan committed May 20, 2024
1 parent eca530c commit 5691c2b
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 20 deletions.
2 changes: 2 additions & 0 deletions examples/genrule/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ genrule(
name = "require_acorn",
srcs = [
":require_acorn_js",
# reference the location where the "acorn" npm package was linked in our root Bazel package.
"//:node_modules/acorn",
"//:node_modules/acorn/dir",
],
outs = ["out2"],
cmd = """
Expand Down
2 changes: 1 addition & 1 deletion npm/private/npm_link_package_store.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def _npm_link_package_store_impl(ctx):
# "node_modules/{package}" so it is available as a direct dependency
root_symlink_path = paths.join("node_modules", package)

files = utils.make_symlink(ctx, root_symlink_path, virtual_store_directory)
files = [utils.make_symlink(ctx, root_symlink_path, virtual_store_directory)]

for bin_name, bin_path in ctx.attr.bins.items():
bin_file = ctx.actions.declare_file(paths.join("node_modules", ".bin", bin_name))
Expand Down
41 changes: 26 additions & 15 deletions npm/private/npm_package_store.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def _npm_package_store_impl(ctx):

src = None
virtual_store_directory = None
transitive_files = []
files = []
direct_ref_deps = {}

npm_package_store_deps = []
Expand Down Expand Up @@ -234,7 +234,7 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
for dep_alias in dep_aliases:
# "node_modules/{virtual_store_root}/{virtual_store_name}/node_modules/{package}"
dep_symlink_path = paths.join("node_modules", utils.virtual_store_root, virtual_store_name, "node_modules", dep_alias)
transitive_files.extend(utils.make_symlink(ctx, dep_symlink_path, dep_virtual_store_directory))
files.append(utils.make_symlink(ctx, dep_symlink_path, dep_virtual_store_directory))
else:
# this is a ref npm_link_package, a downstream terminal npm_link_package
# for this npm dependency will create the dep symlinks for this dep;
Expand All @@ -251,13 +251,12 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
if dep_virtual_store_directory not in linked_virtual_store_directories:
# "node_modules/{virtual_store_root}/{virtual_store_name}/node_modules/{package}"
dep_symlink_path = paths.join("node_modules", utils.virtual_store_root, virtual_store_name, "node_modules", dep_package)
transitive_files.extend(utils.make_symlink(ctx, dep_symlink_path, dep_virtual_store_directory))
files.append(utils.make_symlink(ctx, dep_symlink_path, dep_virtual_store_directory))
npm_package_store_deps.append(store)
else:
# if ctx.attr.src is _not_ set and ctx.attr.deps is, this is a terminal
# package with deps being the transitive closure of deps;
# this pattern is used to break circular dependencies between 3rd
# party npm deps; it is not recommended for 1st party deps
# if ctx.attr.src is _not_ set then this is a terminal 3p package with ctx.attr.deps is
# being the transitive closure of deps; this pattern is used to break circular dependencies
# between 3rd party npm deps; it is not recommended for 1st party deps
deps_map = {}
for dep, _dep_aliases in ctx.attr.deps.items():
dep_package = dep[NpmPackageStoreInfo].package
Expand All @@ -277,8 +276,6 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
if virtual_store_name == dep_virtual_store_name:
# provide the node_modules directory for this package if found in the transitive_closure
virtual_store_directory = dep[NpmPackageStoreInfo].virtual_store_directory
if virtual_store_directory:
transitive_files.append(virtual_store_directory)
for dep_ref_dep, dep_ref_dep_aliases in dep_ref_deps.items():
dep_ref_dep_virtual_store_name = utils.virtual_store_name(dep_ref_dep[NpmPackageStoreInfo].package, dep_ref_dep[NpmPackageStoreInfo].version)
if not dep_ref_dep_virtual_store_name in deps_map:
Expand All @@ -293,9 +290,10 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,
for dep_ref_dep_alias in dep_ref_dep_aliases:
# "node_modules/{virtual_store_root}/{virtual_store_name}/node_modules/{package}"
dep_ref_dep_symlink_path = paths.join("node_modules", utils.virtual_store_root, dep_virtual_store_name, "node_modules", dep_ref_dep_alias)
transitive_files.extend(utils.make_symlink(ctx, dep_ref_dep_symlink_path, dep_ref_def_virtual_store_directory))
files.append(utils.make_symlink(ctx, dep_ref_dep_symlink_path, dep_ref_def_virtual_store_directory))

files = [virtual_store_directory] if virtual_store_directory else []
if virtual_store_directory:
files.append(virtual_store_directory)

npm_package_store_deps.extend([
target[NpmPackageStoreInfo]
Expand All @@ -304,10 +302,23 @@ deps of npm_package_store must be in the same package.""" % (ctx.label.package,

files_depset = depset(files)

transitive_files_depset = depset(files, transitive = [depset(transitive_files)] + [
npm_package_store.transitive_files
for npm_package_store in npm_package_store_deps
])
if ctx.attr.src:
transitive_files_depset = depset(files, transitive = [
npm_package_store.transitive_files
for npm_package_store in npm_package_store_deps
])
else:
# if ctx.attr.src is _not_ set then this is a terminal 3p package with ctx.attr.deps is
# being the transitive closure of deps; this pattern is used to break circular dependencies
# between 3rd party npm deps; it is not recommended for 1st party deps because
# npm_package_store_deps is the transitive closure of all the entire package store deps, we
# can safely add just `files` from each of these to `transitive_files_depset`. Doing so
# reduces the size of `transitive_files_depset` significantly and reduces analysis time and
# Bazel memory usage during analysis
transitive_files_depset = depset(files, transitive = [
npm_package_store.files
for npm_package_store in npm_package_store_deps
])

providers = [
DefaultInfo(
Expand Down
5 changes: 1 addition & 4 deletions npm/private/utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,6 @@ def _virtual_store_name(name, version):
return "%s@%s" % (escaped_name, escaped_version)

def _make_symlink(ctx, symlink_path, target_file):
files = []
if not is_bazel_6_or_greater():
# ctx.actions.declare_symlink was added in Bazel 6
fail("A minimum version of Bazel 6 required to use rules_js")
Expand All @@ -286,9 +285,7 @@ def _make_symlink(ctx, symlink_path, target_file):
output = symlink,
target_path = relative_file(target_file.path, symlink.path),
)
files.append(target_file)
files.append(symlink)
return files
return symlink

def _parse_package_name(package):
# Parse a @scope/name string and return a (scope, name) tuple
Expand Down

0 comments on commit 5691c2b

Please sign in to comment.