I/O and memory diagnostic improvements in DistributedClosestPoint driver - #1952
I/O and memory diagnostic improvements in DistributedClosestPoint driver#1952kennyweiss wants to merge 16 commits into
Conversation
| auto& req = isendRequests.back(); | ||
| relay::mpi::isend_using_schema(*xferNodes[m_rank], firstRecipForMyQuery, tag, m_mpiComm, &req); | ||
| // isend_using_schema() deep-copies into the request buffer. | ||
| xferNodes.erase(m_rank); |
There was a problem hiding this comment.
These three xferNodes.erase() seem to save a lot of memory during computeClosestPoint in hip runs
| input | branch | query time | RSS peak | RSS post-query | RSS after trim |
|---|---|---|---|---|---|
100k |
develop | 2.16 s | 3.76 GiB | 3.76 GiB | 3.57 GiB |
100k |
branch | 2.14 s | 3.64 GiB | 3.64 GiB | 3.55 GiB |
1M |
develop | 22.91 s | 5.97 GiB | 5.38 GiB | 4.33 GiB |
1M |
branch | 23.23 s | 4.77 GiB | 4.27 GiB | 4.13 GiB |
10M |
develop | 286.76 s | 27.81 GiB | 9.04 GiB | 9.00 GiB |
10M |
branch | 285.99 s | 15.89 GiB | 9.04 GiB | 9.00 GiB |
Notes:
- In the 10M case, the peak memory went down from ~28 GB to ~16GB with this change.
malloc_trimonly helped marginally indevelopand even less after the changes in this file- This was in a Debug config, so don't read too much into the query times.
There was a problem hiding this comment.
Nice memory reduction for larger problem sizes.
There was a problem hiding this comment.
Thanks. I think it might essentially be the same fix as #1889, which hasn't merged yet.
Tag: @MrBurmark
There was a problem hiding this comment.
Indeed reducing the lifespan of the transfer nodes is the essence of #1889.
There was a problem hiding this comment.
@kennyweiss are you planning to collect data on the post #1948 develop?
There was a problem hiding this comment.
@MrBurmark -- I rebased my branch on axom@develop after merging both of your PRs and removed my node.erase calls.
I reran the tests w/ and w/o dynamic filtering to confirm that your changes captured the memory improvements (w/ amdclang++ / rocm@7.2.1):
| input | dynamic filtering | query time | RSS peak | RSS post-query | RSS after trim |
|---|---|---|---|---|---|
1M |
enabled | 2.318 s | 4.67 GiB | 4.16 GiB | 4.03 GiB |
1M |
disabled | 2.300 s | 4.66 GiB | 4.16 GiB | 4.00 GiB |
10M |
enabled | 25.588 s | 15.78 GiB | 8.93 GiB | 8.89 GiB |
10M |
disabled | 27.196 s | 15.78 GiB | 8.93 GiB | 8.89 GiB |
Note: the previous timings were in a Debug (-O0) config and these are in a DebWithRelInfo (-O2), so don't read too much into the timings changes.
There was a problem hiding this comment.
Thanks for rerunning. AFAIK, codes don't put debug builds out for users 😆
There was a problem hiding this comment.
(I used a Debug config for the initial run since I was focusing on the memory rather than the timings.)
|
Note: The failing github actions CI job (gcc@13.3.1, shared, 32 bit) is related using a 64-bit type in a 32-bit build. Should be easy to fix. |
rhornung67
left a comment
There was a problem hiding this comment.
@kennyweiss thank you for this.
964752c to
7404b58
Compare
When provided, we use that instead of an analytically generated mesh.
Allow for topologies other than "mesh" and for unstructured topologies.
And for input meshes without a "fields" group.
... and remove analytical generation from DCP example.
Also adds troubleshooting sections about shroud and about symlinked paths.
The test meshes match the previous analytically generated circle/sphere and are validated.
* In convert_sidre_protocol, run w/ correct protocol * In new point mesh generator script, ensure MPI/conduit are available when we expect them * Fix check for multi-domain runs
14519a2 to
f97c476
Compare
Summary
malloc_trimhelps clear memoryMisc: