[PATCH] D133083: [support] Allow TimeProfiler tracing across threads

Thu Sep 1 07:13:33 PDT 2022

russell.gallop added a subscriber: mehdi_amini.
russell.gallop added a comment.

Hi @mbs-modular ,

This looks interesting. Does this help with the case @mehdi_amini outlined here: https://reviews.llvm.org/D118550/new/#3323665 ? It would be great to be able to get the TimeProfile to trace anything multi-threaded in LLVM (anything using Thread Pool), rather than having to hook it into each use.

Regarding the patch, it looks like there are a number of non-functional changes here (e.g. move and rename Entry class, making Detail optional for TimeTraceScope, some style and formatting and a few others). Would it be possible to split those out to to a separate NFC patch so there is a change focussed on the new functionality? I'm also trying to think whether the change to high_resolution_clock might have side-effects so could be a separate patch. I feel like it's the kind of thing that might trip up on one platform or another.

> Could not find unit tests, please let me know if I missed them.

I'm not aware of any unit tests (though they sound like a good idea :-) ). There are some lit tests in clang/test/Driver and lld/test.

Likely unrelated but there is an outstanding bug on time-trace (https://github.com/llvm/llvm-project/issues/56554). Just noting it here in case anything here either fixes that or desensitizes it.

Regards
Russ

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133083/new/

https://reviews.llvm.org/D133083