[PATCH] D86406: Speedup llvm-dwarfdump 3.9x

Tue Aug 25 00:25:52 PDT 2020

jankratochvil added a comment.

In D86406#2234381 <https://reviews.llvm.org/D86406#2234381>, @JDevlieghere wrote:

> If I understand the patch correctly, the speedup is gained by caching the mode instead of trying to recompute it every time we call `colorsEnabled()`?

Yes.

> This seems like a huge speedup for such a small thing, maybe I'm missing something?

Detecting whether fd 1 supports colors means an ioctl() call which is a kernel call. That is several orders of magnitue more expensive than any in-process codeflow and in-process calculations. This is on Linux, I did not check OSX.

> I don't think we should move it into the stream, if that's what you mean by "there". The color `cl::opt` is part of WithColor so we should check its value there.

I plan to make raw_ostream to have auto/on/off mode (default=auto). WithColor's ctor will set the mode of the raw_ostream. raw_ostream will do the expensive ioctl() detection only during first time changing color if raw_ostream has auto mode and raw_ostream will cache the detected value.

(Not yet written, busy with DWZ stuff, thanks for asking.)

> Can we achieve the same thing by resolving the ColorMode to `Enable` and `Disable` in the WithColor constructor?

No because WithColor is ctored/dtored many times for every DIE.  This is what results in so many expensive ioctl()s.

It could be done in WithColor if its Mode is changed to a static member but then that static member would be set according to current raw_ostream and it would be incorrect if WithColor's static member is later used with different raw_ostream.

In D86406#2235331 <https://reviews.llvm.org/D86406#2235331>, @jhenderson wrote:

> Maybe I misunderstood what this bit of the strace output was complaining about then:
> `ioctl(1, TCGETS, 0x7ffd64d7f340)        = -1 ENOTTY (Inappropriate ioctl for device)`
> (I have never used strace, so was only guessing based on its general appearance). I took it to mean that an attempt was made to change the colour, but failed because files don't support different colours, and that this failure is expensive.

Expensive is the detection. Color output is just printing some bytes like "\x1B[0;36m".

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86406