sepavloff marked 2 inline comments as done.
sepavloff added inline comments.
================
Comment at: clang/lib/Driver/Driver.cpp:3782
+ = Cmd.getProcessStatistics();
+ if (ProcStat) {
+ if (PrintProcessStat) {
----------------
aganea wrote:
> In the case where `!ProcStat`, I am wondering if we shouldn't emit zero
> values, in the report file at least. Otherwise if an invocation fails, it
> won't be there in the report and we might wonder why. Emitting 0 might
> indicate that something went wrong.
The feature allows getting statistics using conventional tools like `make`. In
this case the absence of numbers for a particular file is not an issue, maybe
the file has already been built. If something goes wrong, clang must handle it
in `Compilation::ExecuteCommand` and react properly. The fact of failed
invocation would be handled by build tool. For the purpose of monitoring
performance numbers it is more convenient to record successful compilations
only.
================
Comment at: clang/lib/Driver/Driver.cpp:3814
+ llvm::raw_fd_ostream OS(StatReportFile, EC,
llvm::sys::fs::OF_Append);
+ if (!EC) {
+ if (auto L = OS.tryToLock())
----------------
aganea wrote:
> If the goal is to report accurate information, maybe it's worth looping here
> a bit in case of an error, to give the chance to other clang.exe instances to
> release the file lock? What do you think? (same for `tryToLock`)
Actually this method makes blocking request and waits infinitely.
There is long discussion of how this lock helper must be implemented:
https://reviews.llvm.org/D79066. Finally the variant with timeout was removed,
only blocking one remained. The method has prefix `try` because it requires
handling result value.
If you think the name or interface might be better, you can participate in the
discussion in D79066, that patch isn't committed yet.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78903/new/
https://reviews.llvm.org/D78903
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
