Skip to content

Commit

Permalink
rust_analyzer: don't build a tree of RustAnalyzerInfos (#3028)
Browse files Browse the repository at this point in the history
This patch adds an `id` to RustAnalyzerInfo, and replace all the
recursive RustAnalyzerInfos with the respective crate `id`s.
This fixes RustAnalyzerInfos becoming exponentially expensive to build
in the presence of aliases.

---

Before this patch, `RustAnalyzerInfo.deps` contains the RustAnalyzerInfo
of the target crate's deps, and so on recursively.
This forms a graph of Infos where there is one Info per target, but
multiple paths from one target to another.
e.g. these could all point to `bar`: `foo.deps[0] == foo.deps[1].deps[0]
== foo.aliases[0]`.

If we walk `foo` as a tree, we will see `bar` multiple times. Two
operations that do this are `print(info)` and `{info: 42}` which hashes
the object. As the graph grows, the number of paths grows
combinatorically.

`RustAnalyzerInfo.aliases` is defined as a dict from `RustAnalyzerInfo
=> string`, so building this triggers the slowdown.

It would be possible to fix this by e.g. replacing this dict with a list
of pairs.
However the work `rust_analyzer_aspect` does is fundamentally local and
does not need a recursive data structure. Eliminating it means we can
freely use these values as keys, print them, etc.

---

Timings for `ra_ap_rust-analyzer` (which heavily aliases its deps):

```
blaze build //third_party/bazel_rules/rules_rust/tools/rust_analyzer:gen_rust_project; \
time blaze run //third_party/bazel_rules/rules_rust/tools/rust_analyzer:gen_rust_project -- //third_party/rust/ra_ap_rust_analyzer/...

===Before===
Executed in  211.06 secs    fish           external
   usr time    0.24 secs    0.00 micros    0.24 secs
   sys time    1.24 secs  604.00 micros    1.24 sec

===After===
Executed in    3.24 secs    fish           external
   usr time    0.15 secs  389.00 micros    0.15 secs
   sys time    1.18 secs  125.00 micros    1.18 secs
```
  • Loading branch information
sam-mccall authored Dec 5, 2024
1 parent 6cfd508 commit 443b089
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 46 deletions.
8 changes: 7 additions & 1 deletion extensions/prost/private/prost.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,10 @@ def _rust_prost_aspect_impl(target, ctx):
# https://github.com/rust-analyzer/rust-analyzer/blob/2021-11-15/crates/project_model/src/workspace.rs#L529-L531
cfgs = ["test", "debug_assertions"]

crate_id = "prost-" + dep_variant_info.crate_info.root.path

rust_analyzer_info = write_rust_analyzer_spec_file(ctx, ctx.rule.attr, ctx.label, RustAnalyzerInfo(
id = crate_id,
aliases = {},
crate = dep_variant_info.crate_info,
cfgs = cfgs,
Expand Down Expand Up @@ -332,7 +335,10 @@ def _rust_prost_library_impl(ctx):
transitive = transitive,
),
),
RustAnalyzerGroupInfo(deps = [proto_dep[RustAnalyzerInfo]]),
RustAnalyzerGroupInfo(
crate_specs = proto_dep[RustAnalyzerInfo].crate_specs,
deps = proto_dep[RustAnalyzerInfo].deps,
),
]

rust_prost_library = rule(
Expand Down
8 changes: 5 additions & 3 deletions rust/private/providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -157,20 +157,22 @@ TestCrateInfo = provider(
RustAnalyzerInfo = provider(
doc = "RustAnalyzerInfo holds rust crate metadata for targets",
fields = {
"aliases": "Dict[RustAnalyzerInfo, String]: Replacement names these targets should be known as in Rust code",
"aliases": "Dict[String, String]: Maps crate IDs to Replacement names these targets should be known as in Rust code",
"build_info": "BuildInfo: build info for this crate if present",
"cfgs": "List[String]: features or other compilation `--cfg` settings",
"crate": "CrateInfo: Crate information.",
"crate_specs": "Depset[File]: transitive closure of OutputGroupInfo files",
"deps": "List[RustAnalyzerInfo]: direct dependencies",
"deps": "List[String]: IDs of direct dependency crates",
"env": "Dict[String: String]: Environment variables, used for the `env!` macro",
"id": "String: Arbitrary unique ID for this crate",
"proc_macro_dylib_path": "File: compiled shared library output of proc-macro rule",
},
)

RustAnalyzerGroupInfo = provider(
doc = "RustAnalyzerGroupInfo holds multiple RustAnalyzerInfos",
fields = {
"deps": "List[RustAnalyzerInfo]: direct dependencies",
"crate_specs": "Depset[File]: transitive closure of OutputGroupInfo files",
"deps": "List[String]: crate IDs of direct dependencies",
},
)
75 changes: 33 additions & 42 deletions rust/private/rust_analyzer.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def write_rust_analyzer_spec_file(ctx, attrs, owner, base_info):
cfgs = base_info.cfgs,
env = base_info.env,
deps = base_info.deps,
id = base_info.id,
crate_specs = depset(direct = [crate_spec], transitive = [base_info.crate_specs]),
proc_macro_dylib_path = base_info.proc_macro_dylib_path,
build_info = base_info.build_info,
Expand All @@ -72,21 +73,6 @@ def write_rust_analyzer_spec_file(ctx, attrs, owner, base_info):

return rust_analyzer_info

def _accumulate_rust_analyzer_info(dep_infos_to_accumulate, label_index_to_accumulate, dep):
if dep == None:
return
if RustAnalyzerInfo in dep:
label_index_to_accumulate[dep.label] = dep[RustAnalyzerInfo]
dep_infos_to_accumulate.append(dep[RustAnalyzerInfo])
if RustAnalyzerGroupInfo in dep:
for expanded_dep in dep[RustAnalyzerGroupInfo].deps:
label_index_to_accumulate[expanded_dep.crate.owner] = expanded_dep
dep_infos_to_accumulate.append(expanded_dep)

def _accumulate_rust_analyzer_infos(dep_infos_to_accumulate, label_index_to_accumulate, deps_attr):
for dep in deps_attr:
_accumulate_rust_analyzer_info(dep_infos_to_accumulate, label_index_to_accumulate, dep)

def _rust_analyzer_aspect_impl(target, ctx):
if (rust_common.crate_info not in target and
rust_common.test_crate_info not in target and
Expand All @@ -107,41 +93,55 @@ def _rust_analyzer_aspect_impl(target, ctx):
cfgs += [f[6:] for f in ctx.rule.attr.rustc_flags if f.startswith("--cfg ") or f.startswith("--cfg=")]

build_info = None
dep_infos = []
labels_to_rais = {}

for dep in getattr(ctx.rule.attr, "deps", []):
# Save BuildInfo if we find any (for build script output)
if BuildInfo in dep:
build_info = dep[BuildInfo]

_accumulate_rust_analyzer_infos(dep_infos, labels_to_rais, getattr(ctx.rule.attr, "deps", []))
_accumulate_rust_analyzer_infos(dep_infos, labels_to_rais, getattr(ctx.rule.attr, "proc_macro_deps", []))

_accumulate_rust_analyzer_info(dep_infos, labels_to_rais, getattr(ctx.rule.attr, "crate", None))
_accumulate_rust_analyzer_info(dep_infos, labels_to_rais, getattr(ctx.rule.attr, "actual", None))
# Gather required info from dependencies.
label_to_id = {} # {Label of dependency => crate_id}
crate_specs = [] # [depset of File - transitive crate_spec.json files]
attrs = ctx.rule.attr
all_deps = getattr(attrs, "deps", []) + getattr(attrs, "proc_macro_deps", []) + \
[dep for dep in [getattr(attrs, "crate", None), getattr(attrs, "actual", None)] if dep != None]
for dep in all_deps:
if RustAnalyzerInfo in dep:
label_to_id[dep.label] = dep[RustAnalyzerInfo].id
crate_specs.append(dep[RustAnalyzerInfo].crate_specs)
if RustAnalyzerGroupInfo in dep:
for expanded_dep in dep[RustAnalyzerGroupInfo].deps:
label_to_id[expanded_dep] = expanded_dep
crate_specs.append(dep[RustAnalyzerGroupInfo].crate_specs)

deps = label_to_id.values()
crate_specs = depset(transitive = crate_specs)

if rust_common.crate_group_info in target:
return [RustAnalyzerGroupInfo(deps = dep_infos)]
return [RustAnalyzerGroupInfo(deps = deps, crate_specs = crate_specs)]
elif rust_common.crate_info in target:
crate_info = target[rust_common.crate_info]
elif rust_common.test_crate_info in target:
crate_info = target[rust_common.test_crate_info].crate
else:
fail("Unexpected target type: {}".format(target))

aliases = {}
for aliased_target, aliased_name in getattr(ctx.rule.attr, "aliases", {}).items():
if aliased_target.label in labels_to_rais:
aliases[labels_to_rais[aliased_target.label]] = aliased_name
aliases = {
label_to_id[target.label]: name
for (target, name) in getattr(attrs, "aliases", {}).items()
if target.label in label_to_id
}

# An arbitrary unique and stable identifier.
crate_id = "ID-" + crate_info.root.path

rust_analyzer_info = write_rust_analyzer_spec_file(ctx, ctx.rule.attr, ctx.label, RustAnalyzerInfo(
id = crate_id,
aliases = aliases,
crate = crate_info,
cfgs = cfgs,
env = crate_info.rustc_env,
deps = dep_infos,
crate_specs = depset(transitive = [dep.crate_specs for dep in dep_infos]),
deps = deps,
crate_specs = crate_specs,
proc_macro_dylib_path = find_proc_macro_dylib_path(toolchain, target),
build_info = build_info,
))
Expand Down Expand Up @@ -193,14 +193,6 @@ _WORKSPACE_TEMPLATE = "__WORKSPACE__/"
_EXEC_ROOT_TEMPLATE = "__EXEC_ROOT__/"
_OUTPUT_BASE_TEMPLATE = "__OUTPUT_BASE__/"

def _crate_id(crate_info):
"""Returns a unique stable identifier for a crate
Returns:
(string): This crate's unique stable id.
"""
return "ID-" + crate_info.root.path

def _create_single_crate(ctx, attrs, info):
"""Creates a crate in the rust-project.json format.
Expand All @@ -214,8 +206,7 @@ def _create_single_crate(ctx, attrs, info):
"""
crate_name = info.crate.name
crate = dict()
crate_id = _crate_id(info.crate)
crate["crate_id"] = crate_id
crate["crate_id"] = info.id
crate["display_name"] = crate_name
crate["edition"] = info.crate.edition
crate["env"] = {}
Expand Down Expand Up @@ -263,8 +254,8 @@ def _create_single_crate(ctx, attrs, info):
# There's one exception - if the dependency is the same crate name as the
# the crate being processed, we don't add it as a dependency to itself. This is
# common and expected - `rust_test.crate` pointing to the `rust_library`.
crate["deps"] = [_crate_id(dep.crate) for dep in info.deps if _crate_id(dep.crate) != crate_id]
crate["aliases"] = {_crate_id(alias_target.crate): alias_name for alias_target, alias_name in info.aliases.items()}
crate["deps"] = [id for id in info.deps if id != info.id]
crate["aliases"] = info.aliases
crate["cfg"] = info.cfgs
toolchain = find_toolchain(ctx)
crate["target"] = (_EXEC_ROOT_TEMPLATE + toolchain.target_json.path) if toolchain.target_json else toolchain.target_flag_value
Expand Down

0 comments on commit 443b089

Please sign in to comment.