[PATCH] D87272: [lld] Buffer writes when composing a single diagnostic

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D87272#2264383 <https://reviews.llvm.org/D87272#2264383>, @dblaikie wrote:

> In D87272#2263758 <https://reviews.llvm.org/D87272#2263758>, @MaskRay wrote:
>
>> In D87272#2261876 <https://reviews.llvm.org/D87272#2261876>, @dblaikie wrote:
>>
>>> In D87272#2261812 <https://reviews.llvm.org/D87272#2261812>, @MaskRay wrote:
>>>
>>>> In D87272#2261768 <https://reviews.llvm.org/D87272#2261768>, @dblaikie wrote:
>>>>
>>>>> Is performance important in this case? (admittedly, some folks might have builds full of warnings they aren't paying attention to & so they still care about performance then?) Generally the idea with clang/etc is that anything that produces user output (other than the successfully compiled/linked file) doesn't need to be fast (in compile/link-time terms) because it'll take /way/ longer for a user to read and think about the error message than the slight increase in execution time.
>>>>
>>>> The nicer thing is that when running under strace/truss, there will be just one write for one diagnostic.
>>>
>>> Is that true of many other tools? (I wouldn't've expected it'd be true of clang, for instance - or probably other LLVM utilities) - is it especially helpful?
>>
>> The motivation was to reduce unneeded ::write calls.
>
> Sounds like unnecessary/premature optimization, though?

I don't think it is premature optimization. When a --symbol-ordering-file has absent entries or when --call-graph-ordering-file has incorrect entries, the terminal output can be quite slow. This change can make it much faster by also making things just better (one write syscall for one line of diagnostic).

You can save 2 or 3 lines by not making this change, but to me the "mental complexity" is larger: a lock is needed to prevent interleaving output from multiple threads.

>> Now with jhenderson's suggestion (thanks), this turned to a refactoring. The total number of lines has increased but it looks clearer (the log/warn/error functions don't have to deal with lock_guard, sep, and the order), so I think the end result is not bad.
>
> Seems like the code could be simpler still by doing that refactoring without adding the buffering?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87272