Re: [Libreoffice] Assertions and Logging

On Tuesday 22 of November 2011, Stephan Bergmann wrote:
> On 11/22/2011 05:17 PM, Lubos Lunak wrote:
...
> I did not commit it in order to stop any discussion.  Sorry if it looked
> that way.  Rather, as I did not get any totally disagreeing reactions, I
> thought it would be easier to polish this further if the code is
> actually in.  (And I think it was a good decision to commit it early;
> naturally, the various tinderboxes pointed me to some flaws that were
> still in.)

 Well, ok, what I had in mind was you having to rework what has been already 
committed in a case somebody points out something that requires extensive 
changes (or the possibility of "it's already in" being used as an argument 
against the changes), but if you see it this way, fair enough.

> >   Does it really matter, when it is used only in debug builds anyway?
> > Debug builds are slower already anyway, and if the cost of generating the
> > output is considered expensive, what about the cost of wherever the
> > output will end up?
>
> While the macros are currently only enabled in debug builds, I see no
> reason that should always be so.  A production build from which one can
> extract such logging information would be quite useful, I think.  So I
> designed the macros with that in mind.

 I still don't think that changes much. If the output is already generated, it 
will end up somewhere, and that's either /dev/null, in which case the 
generation should not be enabled at all, or it's some terminal, where the 
cost of scrolling, displaying, etc. is much higher than just some string 
creation, or it's some log file, which involves system buffers, writing to 
disk, etc. and that is again more expensive. We could probably make each of 
the output calls solve a random sudoku just for the fun of it and it wouldn't 
make a noticeable difference :). This usage would need at least the part for 
deciding whether to generate any output to be cheap though.

> Maybe it would make sense to simplify the non-S variants so that they
> only work with a plain string, removing the printf-style format feature.
>   But that would mean some work, adapting the existing uses of OSL_TRACE
> that depend on that feature.

 That looks like a good reason to keep the printf variant, but not as the 
primary one.

> The only problem with the S variants is the call-site bloat of creating
> a std::ostringstream and piping into it, etc.  I don't see how that can
> elegantly be avoided, though.

 If the std::stream stuff is really that heavy, then we don't necessarily need 
to use it (although I admit that this may not fit the above requirement if 
doing the work of using something else is considered not elegant). The stream 
<< foo style does not mandate using std::stream, e.g. QDebug [*] uses this 
style too. And this could be done later, if it actually is needed. I think 
it's better to start with good API and possibly fix the performance later 
rather than start with good performance and fix the API later.

 As for the plain string case being fast, maybe that could be achieved using 
an overload (or template) with std::stream and const char*.


[*] http://doc.qt.nokia.com/4.7/qdebug.html
-- 
 Lubos Lunak
 l.lunak@suse.cz