Skip to content

Commit

Permalink
[engine] Improving performance by iteratively hydrate build file addr…
Browse files Browse the repository at this point in the history
…esses within SelectTransitive (pantsbuild#4349)

### Problem

As described in pantsbuild#4283, the issue with recursively executing `SelectTransitive` is lots of calls to `store_list` creating massive intermediate lists that increases memory footprint as well as impacts perf.

### Solution

Same idea as in pantsbuild#4265 using `loop_fn` rewrite `SelectTransitive` to execute iteratively. One small difference is in order to use `OrderMap` and `HashSet` for state tracking and outputs are in `Value`s, those `Values` are turned into `Key`s.

ps: previous PR pantsbuild#4334 paved way for this optimization by splitting `SelectTransitive` from `SelectDependencies(...transitive=true)`.

### Result

All existing tests pass. Have verified there is only one `ST` node through viz.

Two follow up items
* pantsbuild#4358 reenable cycle detection
* pantsbuild#3925 Remove BFA now not only is a clean up task, but has performance benefits because the second round hydration is on `Address` which is redundant.
  • Loading branch information
peiyuwang authored and lenucksi committed Apr 22, 2017
1 parent ec12f22 commit 861a746
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 114 deletions.
4 changes: 2 additions & 2 deletions src/python/pants/engine/legacy/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from pants.base.exceptions import TargetDefinitionException
from pants.base.parse_context import ParseContext
from pants.base.specs import SingleAddress
from pants.build_graph.address import Address
from pants.build_graph.address import Address, BuildFileAddress
from pants.build_graph.address_lookup_error import AddressLookupError
from pants.build_graph.build_graph import BuildGraph
from pants.build_graph.remote_sources import RemoteSources
Expand Down Expand Up @@ -333,7 +333,7 @@ def create_legacy_graph_tasks(symbol_table_cls):
(HydratedTargets,
[SelectTransitive(HydratedTarget,
Addresses,
field_types=(Address,))],
field_types=(BuildFileAddress,))],
HydratedTargets),
(HydratedTarget,
[Select(symbol_table_constraint),
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/engine/subsystem/native_engine_version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1749afda2a8990fd6371be4423dca0f37ad65379
1c6344ab2dcdbddd692cc4afa7885e4e286f12ba
112 changes: 54 additions & 58 deletions src/rust/engine/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/rust/engine/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ fnv = "1.0.5"
futures = { git = "https://github.com/alexcrichton/futures-rs", branch = "master" }
futures-cpupool = { git = "https://github.com/alexcrichton/futures-rs", branch = "master" }
glob = "0.2.11"
ignore = "0.1.7"
ignore = "0.1.8"
lazy_static = "0.2.2"
ordermap = "0.2.7"
ordermap = "0.2.8"
petgraph = "0.4.4"
# TODO: Waiting for a 0.4.11 release: https://github.com/alexcrichton/tar-rs/pull/99
tar = { git = "https://github.com/alexcrichton/tar-rs", branch = "master" }
Expand Down
Loading

0 comments on commit 861a746

Please sign in to comment.