Skip to content
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.

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

Triage high memory usage when running many tests concurrently #12662

Closed
stuhood opened this issue Aug 26, 2021 · 5 comments
Closed

Triage high memory usage when running many tests concurrently #12662

stuhood opened this issue Aug 26, 2021 · 5 comments
Labels

Comments

@stuhood
Copy link
Sponsor Member

stuhood commented Aug 26, 2021

./pants test :: uses a surprising amount of memory: >1GB for ~200 tests has been observed, with ~400MB for ./pants dependencies --transitive :: in the same repository.

@stuhood stuhood added the bug label Aug 26, 2021
@stuhood stuhood self-assigned this Aug 26, 2021
@stuhood
Copy link
Sponsor Member Author

stuhood commented Nov 5, 2021

We don’t have any built-in facilities for this, so the best bet currently is a combination of native and python memory profiling tools.

To tackle this more deeply though, I expect that we’ll need to add support to pants for reporting the top-memory-consuming Graph nodes (both in terms of their native/Rust memory usage, and the usage of the Python objects that they hold), followed by some optimization of whatever that ends up showing.

@stuhood
Copy link
Sponsor Member Author

stuhood commented Nov 17, 2021

Some good news: it looks like the Rust dhat crate should now work with Pants: nnethercote/dhat-rs#15 ... I haven't been able to try it out yet, but I prototyped integrating it.

EDIT: Almost! nnethercote/dhat-rs#19

@stuhood
Copy link
Sponsor Member Author

stuhood commented Feb 27, 2022

#14638 implements the memory analysis tooling I mentioned above.

There are some peculiarities immediately obvious with the new --memory-summary flag enabled, but investigating them is very parallelizable, and so I've only focused on getting #14638 landed and into the 2.10.x branch so that we can gather data in more contexts.

stuhood added a commit that referenced this issue Feb 27, 2022
)

As described in #12662, some use cases have surprising memory usage. To enable tracking those cases down, this change adds a `--memory-summary` option which summarizes the deep sizes of live objects in the `Graph`.

On the Python side, the deep size is calculated using a very basic deduping walk of `gc.get_referents` (after having investigated [pympler](https://pypi.org/project/Pympler/), [guppy3](https://pypi.org/project/guppy3/), [objsize](https://pypi.org/project/objsize/)). On the Rust side, the `deepsize` crate is used, with sizes derived for all types reachable from `NodeKey` and `NodeOutput`.

Example output:
```
Memory summary:
64		1		pants.backend.docker.subsystems.dockerfile_parser.DockerfileParser
64		1		pants.backend.docker.subsystems.dockerfile_parser.ParserSetup
64		1		pants.backend.java.dependency_inference.java_parser_launcher.JavaParserCompiledClassfiles
64		1		pants.backend.java.dependency_inference.symbol_mapper.FirstPartyJavaTargetsMappingRequest
<snip>
1588620		957		(native) pants.engine.internals.graph.hydrate_sources
2317920		2195		(native) pants.backend.python.dependency_inference.module_mapper.map_module_to_address
2774760		2434		(native) pants.engine.internals.graph.determine_explicitly_provided_dependencies
4446900		1458		(native) pants.engine.internals.graph.resolve_dependencies
```
stuhood added a commit to stuhood/pants that referenced this issue Feb 27, 2022
…tsbuild#14638)

As described in pantsbuild#12662, some use cases have surprising memory usage. To enable tracking those cases down, this change adds a `--memory-summary` option which summarizes the deep sizes of live objects in the `Graph`.

On the Python side, the deep size is calculated using a very basic deduping walk of `gc.get_referents` (after having investigated [pympler](https://pypi.org/project/Pympler/), [guppy3](https://pypi.org/project/guppy3/), [objsize](https://pypi.org/project/objsize/)). On the Rust side, the `deepsize` crate is used, with sizes derived for all types reachable from `NodeKey` and `NodeOutput`.

Example output:
```
Memory summary:
64		1		pants.backend.docker.subsystems.dockerfile_parser.DockerfileParser
64		1		pants.backend.docker.subsystems.dockerfile_parser.ParserSetup
64		1		pants.backend.java.dependency_inference.java_parser_launcher.JavaParserCompiledClassfiles
64		1		pants.backend.java.dependency_inference.symbol_mapper.FirstPartyJavaTargetsMappingRequest
<snip>
1588620		957		(native) pants.engine.internals.graph.hydrate_sources
2317920		2195		(native) pants.backend.python.dependency_inference.module_mapper.map_module_to_address
2774760		2434		(native) pants.engine.internals.graph.determine_explicitly_provided_dependencies
4446900		1458		(native) pants.engine.internals.graph.resolve_dependencies
```

[ci skip-build-wheels]
stuhood added a commit to stuhood/pants that referenced this issue Feb 27, 2022
…tsbuild#14638)

As described in pantsbuild#12662, some use cases have surprising memory usage. To enable tracking those cases down, this change adds a `--memory-summary` option which summarizes the deep sizes of live objects in the `Graph`.

On the Python side, the deep size is calculated using a very basic deduping walk of `gc.get_referents` (after having investigated [pympler](https://pypi.org/project/Pympler/), [guppy3](https://pypi.org/project/guppy3/), [objsize](https://pypi.org/project/objsize/)). On the Rust side, the `deepsize` crate is used, with sizes derived for all types reachable from `NodeKey` and `NodeOutput`.

Example output:
```
Memory summary:
64		1		pants.backend.docker.subsystems.dockerfile_parser.DockerfileParser
64		1		pants.backend.docker.subsystems.dockerfile_parser.ParserSetup
64		1		pants.backend.java.dependency_inference.java_parser_launcher.JavaParserCompiledClassfiles
64		1		pants.backend.java.dependency_inference.symbol_mapper.FirstPartyJavaTargetsMappingRequest
<snip>
1588620		957		(native) pants.engine.internals.graph.hydrate_sources
2317920		2195		(native) pants.backend.python.dependency_inference.module_mapper.map_module_to_address
2774760		2434		(native) pants.engine.internals.graph.determine_explicitly_provided_dependencies
4446900		1458		(native) pants.engine.internals.graph.resolve_dependencies
```

[ci skip-build-wheels]
stuhood added a commit that referenced this issue Feb 27, 2022
…rrypick of #14638) (#14644)

As described in #12662, some use cases have surprising memory usage. To enable tracking those cases down, this change adds a `--memory-summary` option which summarizes the deep sizes of live objects in the `Graph`.

On the Python side, the deep size is calculated using a very basic deduping walk of `gc.get_referents` (after having investigated [pympler](https://pypi.org/project/Pympler/), [guppy3](https://pypi.org/project/guppy3/), [objsize](https://pypi.org/project/objsize/)). On the Rust side, the `deepsize` crate is used, with sizes derived for all types reachable from `NodeKey` and `NodeOutput`.

Example output:
```
Memory summary:
64		1		pants.backend.docker.subsystems.dockerfile_parser.DockerfileParser
64		1		pants.backend.docker.subsystems.dockerfile_parser.ParserSetup
64		1		pants.backend.java.dependency_inference.java_parser_launcher.JavaParserCompiledClassfiles
64		1		pants.backend.java.dependency_inference.symbol_mapper.FirstPartyJavaTargetsMappingRequest
<snip>
1588620		957		(native) pants.engine.internals.graph.hydrate_sources
2317920		2195		(native) pants.backend.python.dependency_inference.module_mapper.map_module_to_address
2774760		2434		(native) pants.engine.internals.graph.determine_explicitly_provided_dependencies
4446900		1458		(native) pants.engine.internals.graph.resolve_dependencies
```

[ci skip-build-wheels]
@stuhood
Copy link
Sponsor Member Author

stuhood commented Feb 28, 2022

A quick summary of why what --memory-summary currently shows is suspicious. The data from a recent run of ./pants test :: in pantsbuild/pants looks like: https://gist.github.com/stuhood/61b8c451a3b3ad3fa0cb6a0eadcd5914. The first column is total bytes used, and the second column is count of objects.

As described on #14638: these are de-duplicated "deep" sizes for these objects. Items from the rust side (items prefixed with (native)) do not count any of Python's held memory: just the deep size of the NodeKey and NodeOutput structs, with any Python references ignored. So there is no overlap between the sizes reported for Python and Rust.

That means that the (native) entries are taking up a very surprising amount of memory right now. For example: ~1.5kb per (native) pants.engine.internals.graph.hydrate_sources entry.

To triage this, someone should poke at adding debug output that uses deep_size_of (generated by #[derive(DeepSizeOf)]) on fields of the relevant NodeKey and NodeOutput instances to see which portion is taking up so much space.

stuhood added a commit that referenced this issue Mar 3, 2022
The `--stats-memory-summary` added in #14638/#14652 was [reporting surprisingly large sizes](#12662 (comment))  for native `NodeKey` structs -- even when excluding the actual Python values that they held. Investigation showed that both the `Task` and `Entry` structs were contributing significantly to the size of the `Task` struct.

The [`internment` crate](https://crates.io/crates/internment) used here (and in #14654) is an alternative to giving these values integer IDs. They become pointers to a unique, shared (technically: leaked) copy of the value. They are consequently 1) much smaller, 2) much faster to compare.

The `top`-reported memory usage of `./pants dependencies --transitive ::`:
* `313M` before (summary [before.txt](https://github.com/pantsbuild/pants/files/8175461/before.txt))
* `220M` after (summary [after.txt](https://github.com/pantsbuild/pants/files/8175462/after.txt))

[ci skip-build-wheels]
stuhood added a commit to stuhood/pants that referenced this issue Mar 3, 2022
…uild#14683)

The `--stats-memory-summary` added in pantsbuild#14638/pantsbuild#14652 was [reporting surprisingly large sizes](pantsbuild#12662 (comment))  for native `NodeKey` structs -- even when excluding the actual Python values that they held. Investigation showed that both the `Task` and `Entry` structs were contributing significantly to the size of the `Task` struct.

The [`internment` crate](https://crates.io/crates/internment) used here (and in pantsbuild#14654) is an alternative to giving these values integer IDs. They become pointers to a unique, shared (technically: leaked) copy of the value. They are consequently 1) much smaller, 2) much faster to compare.

The `top`-reported memory usage of `./pants dependencies --transitive ::`:
* `313M` before (summary [before.txt](https://github.com/pantsbuild/pants/files/8175461/before.txt))
* `220M` after (summary [after.txt](https://github.com/pantsbuild/pants/files/8175462/after.txt))

[ci skip-build-wheels]
@stuhood
Copy link
Sponsor Member Author

stuhood commented Mar 3, 2022

#13483 and #14683 reduce memory usage by 15% and 30% (respectively), for a combined ~40%. #14683 will be cherry-picked to 2.10.x, but #13483 will need a bit more bake time, and so should come in 2.11.x.

Based on those changes, I'm going to call this closed. Please definitely use the --stats-memory-summary flag in 2.10.0rc2+ to report on any other strange cases you encounter!

@stuhood stuhood closed this as completed Mar 3, 2022
@stuhood stuhood removed their assignment Mar 3, 2022
stuhood added a commit that referenced this issue Mar 3, 2022
…pick of #14683) (#14689)

The `--stats-memory-summary` added in #14638/#14652 was [reporting surprisingly large sizes](#12662 (comment))  for native `NodeKey` structs -- even when excluding the actual Python values that they held. Investigation showed that both the `Task` and `Entry` structs were contributing significantly to the size of the `Task` struct.

The [`internment` crate](https://crates.io/crates/internment) used here (and in #14654) is an alternative to giving these values integer IDs. They become pointers to a unique, shared (technically: leaked) copy of the value. They are consequently 1) much smaller, 2) much faster to compare.

The `top`-reported memory usage of `./pants dependencies --transitive ::`:
* `313M` before (summary [before.txt](https://github.com/pantsbuild/pants/files/8175461/before.txt))
* `220M` after (summary [after.txt](https://github.com/pantsbuild/pants/files/8175462/after.txt))

[ci skip-build-wheels]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

No branches or pull requests

2 participants