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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D87272#2264494 <https://reviews.llvm.org/D87272#2264494>, @MaskRay wrote:

> In D87272#2264430 <https://reviews.llvm.org/D87272#2264430>, @dblaikie wrote:
>
>> In D87272#2264421 <https://reviews.llvm.org/D87272#2264421>, @MaskRay wrote:
>>
>>> 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).
>>
>> How much faster is the performance if you user disables the warning they have chosen not to fix? (if they're fixing it, the time taken to fix it will be a lot longer than the link time penalty)
>
> https://reviews.llvm.org/D87121#2260177 I don't use it as a motivation for the patch. I created this patch because I think it is the right thing to do.

Not sure I follow - the patch description seems to be quite focussed on the performance implications of this change - it sounds like, reading the patch description/commit message that that is the motivation.

Oh, perhaps I see another reason that might be less nuanced: This will probably break color printing on Windows when using the normal Windows color API - because it's a stateful API, not something that can be encoded in the stream: https://github.com/llvm/llvm-project/blob/d3f6972abb9c0ac06ddabf61697754c3c6f5d11b/llvm/lib/Support/raw_ostream.cpp#L513


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87272