Re: Fwd: [ABANDONED] Remove some excessive log formatting

On 21/11/2019 12:47, Luboš Luňák wrote:
> > > > Hm, sounds rather like a misuse of the SAL log facility to me.  (Each
> > > > use of it comes at a cost, so it shouldn't be used too lightly for
> > > > anything but logging unusual events.)
>
>   E.g. currently while writing the Skia VCL backend I've added SAL_INFO to
> pretty much to each of the drawing functions. And of course normally nobody
> wants to see that, but it can be useful if needed, all it takes is setting
> SAL_LOG, and it fits nicely with the code (no #ifdefs or anything). So I
> disagree with it being just for the unusual stuff (that's SAL_WARN).
>
>   Is it really that costly? It's no-op for non-debug builds, and I was under
> the impression that even for debug builds it tries to be fast in the common
> case when it's a no-op. So I'd expect this to be insignificant, especially
> when compared to costs such as using debug mode libstdc++.

My concerns are:

* We may eventually want to enable SAL_WARN/SAL_INFO for production 
builds, at which point the space cost of log message strings might 
become an issue.

* Too much trivial SAL_INFO noise (that was once added ad-hoc and should 
arguably have only been in a developer's local build) can make it 
unnecessarily hard to see the more relevant SAL_INFOs (and log area 
granularity is limited in practice).  Of course, it is hard to tell how 
much of the existing SAL_INFO use falls under that noise category.

When I had originally designed SAL_WARN/SAL_INFO, I had anticipated use 
along the lines of:

* Use SAL_WARN if it is an obviously erroneous unusual event.  Like

>             SAL_WARN_IF(
>                 *ppLibraryUrl == nullptr, "sal.osl", "rtl_string2UString failed");

in osl_getModuleURLFromAddress (sal/osl/unx/module.cxx), where it warns 
if the passed-in ppLibraryUrl cannot be converted to an 8-bit string.

* Use SAL_INFO if it is an unusual event that is not obviously an error. 
 Like

>         SAL_INFO_IF(
>             pLib == nullptr, "sal.osl",
>             "dlopen(" << pModuleName << ", " << rtld_mode << "): "
>                 << dlerror());

in osl_loadModuleAscii (sal/osl/unx/module.cxx), where only the calling 
code will know whether or not it is in an error if the requested module 
does not exist.  (And where the captured dlerror() output is information 
that (a) is not available to the caller, and (b) may be valuable in 
cases where the caller decides that the event is an error.)

I do not claim that this is the only reasonable approach to using 
SAL_WARN/SAL_INFO, and I have seen more liberal use effectively from day 
one.  But I'm always a little concerned, for the reasons outlined above, 
when I come across what looks like overly liberal use of SAL_INFO.