Replace `--flush-on-signal <int>` with `--forward-signal <int>` in timem

NERSC / timemory

Modular C++ Toolkit for Performance Analysis and Logging. Profiling API and Tools for C, C++, CUDA, Fortran, and Python. The C++ template API is essentially a framework to creating tools: it is designed to provide a unifying interface for recording various performance measurements alongside data logging and interfaces to other tools.

https://timemory.readthedocs.io/en/develop/

Other

352 stars 38 forks source link

Replace `--flush-on-signal <int>` with `--forward-signal <int>` in timem #230

Closed hidmic closed 3 years ago

hidmic commented 3 years ago

Follow-up to #225. Turns out that:

on Linux, components such as cpu_util (see times man pages) and peak_rss (see getrusage man pages) depend on children processes being waited on.
--flush-on-signal <int> may result in children processes becoming zombie processes if timem gets the signal but its process group does not or children processes do not terminate on that signal.

This patch simply ensures the signal is forwarded. Whether children processes terminate or not it's up to their implementation. When they do, timem properly finishes waiting on them.

hidmic commented 3 years ago

Aha! Thanks for https://github.com/NERSC/timemory/pull/230/commits/3d05af8f72deea230fb41b3cc9d91407501ed71d. Where's the linter configuration? I did look for it but could not find it.

jrmadsen commented 3 years ago

All the formatting checks are in .github/workflows/formatting.yml. If you have clang-format-9 installed, you can just "build" the format target and it will run clang-format on the source code. The only thing to watch out for is different versions of clang-format can occasionally cause slightly different formatting changes and then cause the formatting checks to fail.

jrmadsen commented 3 years ago

It looks like the CircleCI build is failing bc of an unrelated issue with the gotcha include directories inside the container this runs in so I am going to merge.