[Lldb-commits] [PATCH] D27459: Straw-man proposal for new logging syntax

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Part of the reason I like the current "if (log) log->Printf()" is that it doesn't cost you much if logging isn't enabled. So if you have a log line like:

  if (log)
    log->Printf("%s %s %s", myStringRef1.str().c_str(), myStringRef2.str().c_str(), myStringRef3.str().c_str());

And switch over to using:

  LLDB_LOG() << myStringRef1 << " " << myStringRef2 << " " << myStringRef2;

You end up doing some work with all of the types regardless of wether the logging is enabled. We tend to do pretty heavy logging in places and I really don't want performance impacted (the expression parser emits a full novel worth of information when logging is enabled), nor would I want people to log less because they are worried about affecting performance.

So any solution should have very minimal cost if logging isn't enabled. We also do log channels, so any design will need to include the ability to log to a channel, any channel, or only if all channels are enabled.

The other thing I don't like about streams is they are harder to read over a printf style log string. I wrote all of dwarfdump and used the full power off C++ streams and I regret doing this as all of the code is really hard to read. I also didn't like the fact that streams maintain state and can affect things down the line. If we adopt LLVM style streams I believe they don't suffer from the C++ streams having state issue. But with C++ you can do this:

  std::cout << std::dec << 12;
  std::cout << 24;

And then you add some code in the middle:

  std::cout << std::dec << 12;
  std::cout << std::hex << 0x1234;
  std::cout << 24;

Now your "24" comes out as hex instead of decimal. This means you have to start always specifying your state:

  std::cout << std::dec << 12;
  std::cout << std::hex << 0x1234;
  std::cout << std::dec << 24;

So my main objection is the cost of logging when it isn't enable with this approach. I am going to assume we won't use any state in streams if we do ever make something like this. I would still also like the ability to use Printf in the new logs if we do come up with a solution.

How can be solve the performance problem with this approach?


https://reviews.llvm.org/D27459