[lldb-dev] logging in lldb

[Jim Ingham via lldb-dev]

Yeah, I’m with Jason.  I don’t find the current state of things hard to read or work with.  

The proposed solutions seem a little uglier to me.  I try to avoid macros, to me it makes it look like the code is shouting at you, giving the log lines more prominence then they deserve in the overall flow of the code.  And all upper case is harder to read.  If accumulating the log involves substantial code and the code is all in macros it won’t be debuggable, which is a minor shame.  But it’s not a huge deal since you can move the the code out of the log statement temporarily if you need to debug it.

I don’t see the need to work on this, but if it bugs you and you have the time to do it, the moieties weigh evenly enough that I can’t see a reason to object.

Jim

> On Dec 15, 2016, at 4:01 PM, Jason Molenda <jmolenda at apple.com> wrote:
> 
> Hi Pavel, sorry for not keeping up with the thread, I've been super busy all this week.  I'm not going to object to where this proposal has ended up.  I personally have a preference for the old system but not based on any justifiable reasons.
> 
> 
>> On Dec 15, 2016, at 7:13 AM, Pavel Labath <labath at google.com> wrote:
>> 
>> Just to let you know, I will be on vacation until the end of the year,
>> so probably will not respond to any comments until January. If you
>> have any concerns, do let me know, as I'd like to get started when I
>> get back.
>> 
>> pl
>> 
>> On 13 December 2016 at 16:32, Pavel Labath <labath at google.com> wrote:
>>> Hello again,
>>> 
>>> I'd to get back to the question of unifying llvm's and lldb's logging
>>> mechanisms that Chris asked about. In the way these two are
>>> implemented now, they have a number of similarities, but also a number
>>> of differences. Among the differences, there is one that I think will
>>> be most painful to resolve, so I'll start with that one:
>>> 
>>> I am talking about how to disable logging at compile-time. Currently,
>>> llvm's logging mechanism can be controlled both at runtime and
>>> compile-time. lldb's can be only controlled at runtime. While we may
>>> not want/need the compile-time knob, it is a very hard requirement for
>>> llvm, which tries to squeeze every ounce of performance from the
>>> hardware. So, if we are going to have a common logging API, we will
>>> need to support being compiled without it.
>>> 
>>> This has impact on the kind of syntax we are able to use. I see two
>>> problems here.
>>> 
>>> 1. The first one is that our log patterns are split into independent
>>> parts. Currently the pattern is:
>>> Log *log = GetLogIf(Flag);
>>> ...
>>> if (log) log->Printf(...);
>>> 
>>> The API we (mostly?) converged to above is:
>>> Log *log = GetLogIf(Flag);
>>> ...
>>> LLDB_LOG(log, ...);
>>> 
>>> If we want to compile the logging away, getting rid of the second part
>>> is easy, as it is already a macro. However, for a completely clean
>>> compile, we would need to remove the first part as well. Since
>>> wrapping it in #ifdef would be too ugly, I think the easiest solution
>>> would be to just make it go away completely.
>>> 
>>> The way I understand it, the reason we do it in two steps now is to
>>> make the code fast if logging is off. My proposal here would be to
>>> make the code very fast even without the additional local variable. If
>>> we could use the macro like:
>>> LLDB_LOG(Flag, ...)
>>> where the macro would expand to something like:
>>> if (LLVM_UNLIKELY(Flag & lldb_private::enabled_channels)) log_stuff(...)
>>> where `enabled_channels` is just a global integral variable then the
>>> overhead of a disabled log statement would be three instructions
>>> (load, and, branch), some of which could be reused if we had more
>>> logging statements in a function. Plus the macro could hint the cpu
>>> and compiler to optimize for the "false" case. This is still an
>>> increase over the status quo, where the overhead of a log statement is
>>> one or two instructions, but I am not sure if this is significant.
>>> 
>>> 2. The second, and probably bigger, issue is the one mentioned by
>>> Jason earlier in this thread -- the ability to insert random code into
>>> if(log) blocks. Right writing the following is easy:
>>> if (log) {
>>> do_random_stuff();
>>> log->Printf(...);
>>> }
>>> 
>>> In the first version of the macro, this is still easy to write, as we
>>> don't have to worry about compile-time. But if we need this to go
>>> away, we will need to resort to the same macro approach as llvm:
>>> LLDB_DEBUG( { do_random_stuff(); LLDB_LOG(...); });
>>> Which has all the disadvantages Jason mentioned. Although, I don't
>>> think this has to be that bad, as we probably will not be doing this
>>> very often, and the issues can be mitigated by putting the actual code
>>> in a function, and only putting the function calls inside the macro.
>>> 
>>> 
>>> 
>>> So, my question to you is: Do you think these two drawbacks are worth
>>> sacrificing for the sake of having a unified llvm-wide logging
>>> infrastructure? I am willing to drive this, and implement the llvm
>>> side of things, but I don't want to force this onto everyone, if it is
>>> not welcome. If you do not think this is a worthwhile investment then
>>> I'd rather proceed with the previous lldb-only solution we discussed
>>> above, as that is something I am more passionate above, will already
>>> be a big improvement, and a good stepping stone towards implementing
>>> an llvm-wide solution in the future.
>>> 
>>> Of course, if you have alternative proposals on how to implement
>>> llvm-wide logging, I'd love to hear about it.
>>> 
>>> Let me know what you think,
>>> pavel
>