This is an archive of the discontinued LLVM Phabricator instance.

[Support] Get process statistics in ExecuteAndWait and Wait
ClosedPublic

Authored by sepavloff on Apr 27 2020, 12:08 AM.

Details

Summary

The functions sys::ExcecuteAndWait and sys::Wait now have additional
argument of type pointer to structure, which is filled with process
execution statistics upon process termination. These are total and user
execution times and peak memory consumption. By default this argument is
nullptr so existing users of these function must not change behavior.

Diff Detail

Event Timeline

sepavloff created this revision.Apr 27 2020, 12:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2020, 12:08 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
aganea added a subscriber: aganea.Apr 27 2020, 11:10 AM
aganea added inline comments.
llvm/include/llvm/Support/Program.h
60

I would go for a uint64_t because it's shorter to read (and to parse for the preprocessor), but that's my personal preference.

63

I don't think you need clear(), as stated below.

69

This screams for Optional, see D78903.

llvm/lib/Support/Unix/Program.inc
356

ProcStat is not cleared here, in case of an early exit.

llvm/lib/Support/Windows/Program.inc
407

You can simply say: *ProcStat = {};

sepavloff updated this revision to Diff 266451.May 27 2020, 1:27 AM

Updated patch

  • Use Optional<> to keep ProcessStatistics,
  • Added missing clear,
  • Added unit test.
sepavloff marked 5 inline comments as done.May 27 2020, 1:33 AM
sepavloff added inline comments.
llvm/include/llvm/Support/Program.h
60

No problem. Replaced.

69

Indeed, thank you.

llvm/lib/Support/Unix/Program.inc
356

Added call of reset.

llvm/lib/Support/Windows/Program.inc
407

Used Optional<>::reset() here.

LGTM. Please re-run git clang-format before commit.

aganea accepted this revision.Jun 16 2020, 1:29 PM
This revision is now accepted and ready to land.Jun 16 2020, 1:29 PM
This revision was automatically updated to reflect the committed changes.
hubert.reinterpretcast added inline comments.
llvm/lib/Support/Unix/Program.inc
362

Noting here that the functionality in not available on AIX via this BSD interface. Are there plans to increase dependency on this functionality? As it is, our builds are broken.

sepavloff marked an inline comment as done.Jun 21 2020, 9:29 PM
sepavloff added inline comments.
llvm/lib/Support/Unix/Program.inc
362

I created D82282 with workaround. Could you please check if it works? I don't have access to AIX.

hubert.reinterpretcast marked an inline comment as done.Jun 22 2020, 8:20 AM
hubert.reinterpretcast added inline comments.
llvm/include/llvm/Support/Program.h
60

I believe a comment indicating the unit of measure used with the value (looks like KiB to me) should be added.

llvm/lib/Support/Unix/Program.inc
362

Thank you; I'm checking it out.

hubert.reinterpretcast marked an inline comment as done.Jun 25 2020, 2:44 PM
hubert.reinterpretcast added inline comments.
llvm/include/llvm/Support/Program.h
60
sepavloff marked an inline comment as done.Jun 25 2020, 9:03 PM
sepavloff added inline comments.
llvm/include/llvm/Support/Program.h
60

Thank you!