Re: sal/config.g sal/types.h and sal/osl/rtl includes in general

On Mon, Sep 17, 2012 at 3:27 AM, Stephan Bergmann <sbergman@redhat.com> wrote:
> The point of including sal/config.h first is to make sure there is a place
> for code that needs to be seen very early in every compilation unit.  (A
> good example is the "#ifdef sun" stuff at the end of sal/config.h that was
> vital for compiling with Sun Studio.  Granted, for historical reasons there
> is code in sal/config.h that would arguably better go someplace else.)
>
> sal/types.h just happens to be a header that is needed in many compilation
> units.

any compilation unit that use any scalar type sal_* woudl need it...
iow all of them practically...

cppuhelper/source/findofficepath.c
tools/source/misc/pathutils.cxx
extensions/source/nsplugin/source/so_env.cxx

are the only source I could find that include config.h and not
types.h... and it is not clear at all that it was deliberate.
which impacted 2 static library: libnpsoenv.a and libooopathutils.a

ucbhelper/source/client/contenbrooker.cxx was a false postive as the
content of if is excluded on non ANDROID
vcl/source/app/dbggui.cxx showed-up was a false positive since I did
not build in DBG_UTIL mode... ortherwise sal/types.h would have been
included


> Yeah, the difference between theory and practice.  ;)  (Still, for one,
> config.h probably /does/ effectively get included first most of the time,
> thanks to the various headers that include it first thing,

Yep, actually most of sal's include get included indirectly via
sometime many layer of includes... but sure... always always I agree
config.h end-up being the first actual content parsed


> and for another,
> I think there currently is nothing in config.h that makes it extremely
> problematic if it happens to not be included first thing into some
> compilation unit

That is a flaw, when you want a heder to be first, you got to make
sure that you can't forget about it :-)


> , so enforcing it by modifying hundreds of files probably is
> not worth it right now.)

Well, it is like saying... we have thousand of warnings... so
contemplating -WError is not worth it...
that is a self-fullfilling prophecy...

> I could understand the desire for some lo.h from the point of view of
> performance (though the relevant sub-headers will probably manage to stay in
> main memory anyway) or ease-of-maintenance, but what do you mean with
> "flexibility and certainty"?

having a header that is  guaranteed to be included in every source
first (I mean source not header)

give the flexibility of doing fairly fancy stuff to allow for API
transitions, manage compatibility issue (like faking some api
transparently to the rest of the code etc...)
the 'certainty' comes from the property of the mandatory nature of the
header... and that is ensure by making sure that no source is going to
build without it...
for instance that can be achieved by banning any #include
<sal/types.h> or #<rtl/ustring.hxx> for isntance anywhere but in that
header...
that can be easily checked by script.
the absence of these header means that no non-trivail souce is goign
to compile if it forget to include "lo.h"
we can also script a verification that header do not included it and
that source do include it as a first include...

> > you are right... but that odd behavior of assert.h is actually a
> > reason to hunt down random #include of assert.h, especially in
> > headers, and centralize its use.
>
>
> Why?  The includes of assert.h in headers pertain to uses of assert in
> (inline) functions defined in those headers.  I see no need to complicate
> things by building a protocol on top of plain Standard C assert.h here.

Just because the 'Standard' allow you to shoot yourself in the foot
does not mean it is a good idea.

> > for instance
> > sal/inc/osl/thread.hxx:#include <cassert>
> > sal/inc/rtl/strbuf.hxx:#include <cassert>
> > sal/inc/rtl/string.hxx:#include <cassert>
> > sal/inc/rtl/ustrbuf.hxx:#include <cassert>
> > sal/inc/rtl/ustring.hxx:#include <cassert>
> > sal/textenc/unichars.hxx:#include <cassert>
> >
> > and, see above, these guys are included _a lot_ so assert.h get
> > defined/un-defined/redefined a lot too (just with the 6 include above
> > we are talking about 85K times. or somewhere liek 4 times per
> > source...
> > by centralyzing it at some choke point (sal/config.h if you want) and
> > hunting down random include of assert.h we actually improve things...
>
>
> ...measurably?

yes measurably 0.076875 milli-seconds per #include <assert.h> on my linux box.

or to put in another way: 0.15% of the total build time  (6 second
over 4 cpu, out of 17 minutes elapsed)

****
n_th@tpa10 ~ ()$ cat test_assert.c

#include <assert.h>

#ifdef MANY_ASSERT
#include "many_assert.h" // 80K include assert.h
#endif

int main(int argc, char* argv)
{
    assert(argc > 2);
    return argc;
}



n_th@tpa10 ~ ()$ time gcc test_assert.c

real    0m0.030s
user    0m0.016s
sys     0m0.013s
n_th@tpa10 ~ ()$ time gcc test_assert.c

real    0m0.030s
user    0m0.015s
sys     0m0.014s
n_th@tpa10 ~ ()$ time gcc test_assert.c

real    0m0.030s
user    0m0.021s
sys     0m0.008s
n_th@tpa10 ~ ()$ time gcc -DMANY_ASSERT test_assert.c

real    0m6.190s
user    0m5.612s
sys     0m0.574s
n_th@tpa10 ~ ()$ time gcc -DMANY_ASSERT test_assert.c

real    0m6.204s
user    0m5.612s
sys     0m0.588s
n_th@tpa10 ~ ()$ time gcc -DMANY_ASSERT test_assert.c

real    0m6.139s
user    0m5.562s
sys     0m0.572s
n_th@tpa10 ~ ()$


Norbert