[PATCH] D90010: clang-tidy: Reduce number of stderr write calls

Mon Oct 26 22:16:40 PDT 2020

Hiralo added a comment.

In D90010#2355490 <https://reviews.llvm.org/D90010#2355490>, @dblaikie wrote:

> By the looks of the code, you may want to call SetBufferSize only (do not call SetBuffered after that - or it'll go back to the default buffer size of 0.

Oh! I missed it!

>>>> For example, consider clang-tidy running on 10,000 files... we expect to have minimal number of write calls. With the code-as-is makes 10,000 * 7 = 70,000 stderr write calls with small small chunk of strings!!!
>>>> With proposed changes OR with llvm::errs().setBuffered() set we will see only 10,000 legitimate stderr write calls :)
>>>
>>> Right, but what I mean is "what observable difference do you see" - as a user you aren't counting write calls, you're looking at the wall time, for instance. What difference in user-observable behavior are you seeing/are you trying to address, and how much does this change help that user-observable behavior?
>>>
>>> I don't know too much about clang-tidy, but the statistics output for 10,000 files doesn't seem like it'd be that useful, would it? So maybe the solution is to turn it off, rather than to make it faster? (generally error paths are thought of as not needing to be efficient - because we shouldn't be producing thousands of errors to be consumed, because readers aren't going to read thousands of errors, usually? Are your users/you reading all the output that clang-tidy is generating?)
>>
>> It effectively slows down the processing of files.
>
> Do you have any measurements of how much it slows things down/how much the patch you're proposing speeds things up?

Sorry, don't have stats now.

> In any case, I'd suggest either fixing the errors or, if you've decided the errors are acceptable for some reason, using --quiet to silence the errors you aren't interested in.

I think we tried with --quiet... but let me retry it... 
Meantime, we can pause review of this patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90010