Patchwork bitbake: do not set CCACHE_DISABLE=0

login
register
mail settings
Submitter Enrico Scholz
Date July 21, 2012, 11:55 a.m.
Message ID <1342871746-14583-1-git-send-email-enrico.scholz@sigma-chemnitz.de>
Download mbox | patch
Permalink /patch/32787/
State New
Headers show

Comments

Enrico Scholz - July 21, 2012, 11:55 a.m.
ccache checks for existence of environment; not for its value:

  ---- ccache-3.1.7/ccache.c ---
        if (getenv("CCACHE_DISABLE")) {
                cc_log("ccache is disabled");

Hence, avoid setting of $CCACHE_DISABLE instead of assigning '0'.

Unfortunately, bitbake has a similar behavior when evaluating 'export'
and 'unexport' flags so that this variable must be set in a python
function instead of declaring it in bitbake.conf.

Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
---
 meta/classes/base.bbclass |    8 ++++++++
 meta/conf/bitbake.conf    |    3 ---
 2 files changed, 8 insertions(+), 3 deletions(-)
Richard Purdie - July 22, 2012, 8:41 a.m.
On Sat, 2012-07-21 at 13:55 +0200, Enrico Scholz wrote:
> ccache checks for existence of environment; not for its value:
> 
>   ---- ccache-3.1.7/ccache.c ---
>         if (getenv("CCACHE_DISABLE")) {
>                 cc_log("ccache is disabled");
> 
> Hence, avoid setting of $CCACHE_DISABLE instead of assigning '0'.
> 
> Unfortunately, bitbake has a similar behavior when evaluating 'export'
> and 'unexport' flags so that this variable must be set in a python
> function instead of declaring it in bitbake.conf.
> 
> Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
> ---
>  meta/classes/base.bbclass |    8 ++++++++
>  meta/conf/bitbake.conf    |    3 ---
>  2 files changed, 8 insertions(+), 3 deletions(-)

Why doesn't CCACHE_DISABLE[unexport] = "1" help here? Doesn't the
unexport flag stop this entering the environment?

Cheers,

Richard


> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
> index 1f76023..9f9c803 100644
> --- a/meta/classes/base.bbclass
> +++ b/meta/classes/base.bbclass
> @@ -309,6 +309,14 @@ do_build () {
>  python () {
>      import exceptions, string, re
>  
> +    # Disable ccache explicitly if CCACHE is null since gcc may be a
> +    # symlink of ccache some distributions (e.g., Fedora 17). Please
> +    # note that only the existence of $CCACHE_DISABLE matters; the
> +    # value is ignored.
> +    if d.getVar('CCACHE', True) == '':
> +        d.setVar('CCACHE_DISABLE', True)
> +        d.setVarFlag('CCACHE_DISABLE', 'export', True)
> +
>      # Handle PACKAGECONFIG
>      #
>      # These take the form:
> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> index a8a1665..1bb66ad 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -420,9 +420,6 @@ export PATH
>  ##################################################################
>  
>  CCACHE ??= ""
> -# Disable ccache explicitly if CCACHE is null since gcc may be a symlink
> -# of ccache some distributions (e.g., Fedora 17).
> -export CCACHE_DISABLE ??= "${@[0,1][d.getVar('CCACHE', True) == '']}"
>  # Assign CCACHE_DIR a default value to fix a bug of ccache 3.1.7,
>  # since it would always create CCACHE_DIR/.ccache even if
>  # CCACHE_DISABLE = 1.
Enrico Scholz - July 22, 2012, 9:39 a.m.
Richard Purdie
<richard.purdie-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
writes:

>> ccache checks for existence of environment; not for its value:
>> ...
>> Hence, avoid setting of $CCACHE_DISABLE instead of assigning '0'.
>
> Why doesn't CCACHE_DISABLE[unexport] = "1" help here?

You mean, keeping the

 | export CCACHE_DISABLE ??= "${@[0,1][d.getVar('CCACHE', True) == '']}"

and requesting explicitly that user specifies

 | CCACHE_DISABLE[unexport] = "1"

in his .conf?  Sounds hacky and inconsistent and makes it impossible to
set CCACHE_DISABLE by external environment.


> Doesn't the unexport flag stop this entering the environment?

Perhaps.  In the current bitbake, 'unexport' takes precedence over
'export'.  But is this specified somewhere and will perhaps be changed
in a later version.



Enrico
Richard Purdie - July 22, 2012, 10:32 a.m.
On Sun, 2012-07-22 at 11:39 +0200, Enrico Scholz wrote:
> Richard Purdie
> <richard.purdie-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> writes:
> 
> >> ccache checks for existence of environment; not for its value:
> >> ...
> >> Hence, avoid setting of $CCACHE_DISABLE instead of assigning '0'.
> >
> > Why doesn't CCACHE_DISABLE[unexport] = "1" help here?
> 
> You mean, keeping the
> 
>  | export CCACHE_DISABLE ??= "${@[0,1][d.getVar('CCACHE', True) == '']}"
> 
> and requesting explicitly that user specifies
> 
>  | CCACHE_DISABLE[unexport] = "1"
> 
> in his .conf?  Sounds hacky and inconsistent and makes it impossible to
> set CCACHE_DISABLE by external environment.

The idea is that anyone enabling ccache would inherit the bbclass. The
above could therefore be simplified to a hard ??= 1 which the bbclass
resets and unexports.

> > Doesn't the unexport flag stop this entering the environment?
> 
> Perhaps.  In the current bitbake, 'unexport' takes precedence over
> 'export'.  But is this specified somewhere and will perhaps be changed
> in a later version.

I'll happily take a patch specifying that in the bitbake manual. Other
things would break too if this changed behaviour.

Cheers,

Richard
Enrico Scholz - July 22, 2012, 11:03 a.m.
Richard Purdie
<richard.purdie-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
writes:

>> and requesting explicitly that user specifies
>> 
>>  | CCACHE_DISABLE[unexport] = "1"
>> 
>> in his .conf?  Sounds hacky and inconsistent and makes it impossible to
>> set CCACHE_DISABLE by external environment.
>
> The idea is that anyone enabling ccache would inherit the bbclass.

afais, the ccache.bbclass class is for assigning and cleaning some
(imho) strange CCACHE_DIR only which lowers efficiency significantly.

Normal ccache usage with a single CCACHE_DIR works fine (and much better)
without this class.


> The above could therefore be simplified to a hard ??= 1

for what is the '??=' ?  The value does not matter so it makes no sense
that the user can assign a value.


Enrico
Richard Purdie - July 22, 2012, 11:39 a.m.
On Sun, 2012-07-22 at 13:03 +0200, Enrico Scholz wrote:
> Richard Purdie
> <richard.purdie-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> writes:
> 
> >> and requesting explicitly that user specifies
> >> 
> >>  | CCACHE_DISABLE[unexport] = "1"
> >> 
> >> in his .conf?  Sounds hacky and inconsistent and makes it impossible to
> >> set CCACHE_DISABLE by external environment.
> >
> > The idea is that anyone enabling ccache would inherit the bbclass.
> 
> afais, the ccache.bbclass class is for assigning and cleaning some
> (imho) strange CCACHE_DIR only which lowers efficiency significantly.
> 
> Normal ccache usage with a single CCACHE_DIR works fine (and much better)
> without this class.

There are the following concerns that others have raised over time:

a) That the central ccache directory in the user's homedir can get
filled very easily and this isn't something that most users expect.
b) There is reuse of that directory between different architectures
which isn't desired
c) That a clean of a recipe does not remove the ccache objects
d) That CCACHE_DIR might not exist when ccache is called raising errors
e) that ccache has bugs/risk but making it recipe specific alleviates
some of the risk/contamination issues

I neither claim these are good or bad reasons but they are all dealt
with by the class. Its a question of which issues you want to run into
and what behaviour you want. If you strongly dislike the way CCACHE_DIR
is set, that can be overridden.

Personally speaking, I dislike ccache and would love to just remove all
the code related to it and disable it for everyone. Yes, it has some
performance wins in some corner case situations but it is of marginal
utility IMO.

> > The above could therefore be simplified to a hard ??= 1
> 
> for what is the '??=' ?  The value does not matter so it makes no sense
> that the user can assign a value.

Ok, we can just use = there...

Cheers,

Richard
Enrico Scholz - July 22, 2012, 7:38 p.m.
Richard Purdie
<richard.purdie-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
writes:

>> afais, the ccache.bbclass class is for assigning and cleaning some
>> (imho) strange CCACHE_DIR only which lowers efficiency significantly.
>> 
>> Normal ccache usage with a single CCACHE_DIR works fine (and much better)
>> without this class.
>
> There are the following concerns that others have raised over time:

I use

  export CCACHE_DIR = "${TMPDIR}/cache/ccache"

in my setup.


> a) That the central ccache directory in the user's homedir can get
> filled very easily and this isn't something that most users expect.

not a problem when CCACHE_DIR is below ${TMPDIR}.  Although it could
make sense to use the system CCACHE_DIR for -native recipes.


> b) There is reuse of that directory between different architectures
> which isn't desired

generally it does not hurt; having it too finegrained makes it difficult
to limit diskusage.


> c) That a clean of a recipe does not remove the ccache objects

why should somebody want this for a single CCACHE_DIR?


> d) That CCACHE_DIR might not exist when ccache is called raising
>    errors

we have a 'bitbake' wrapper which does more complicated, pseudo related
tasks already. Creating $CCACHE_DIR won't be a problem.


> e) that ccache has bugs/risk but making it recipe specific alleviates
> some of the risk/contamination issues

Only unsolved problem I am aware of is, that -dbg refers wrong source.
Some packages (e.g. dietlibc) need also patches to deal with ccache but
these are an exception and for out-of-oe interest too.

Else, I am using ccache for a very long time (perhaps 10 years or so)
and can not remember miscompilation or contamination.


> Personally speaking, I dislike ccache and would love to just remove all
> the code related to it and disable it for everyone. Yes, it has some
> performance wins in some corner case situations but it is of marginal
> utility IMO.

Some numbers[1] for a from-scratch build of an image (BB_NUMBER_OF_THREADS=4,
'make -j 2'):

  https://www.cvg.de/people/ensc/metrics-ccache-yes.xml.gz
  https://www.cvg.de/people/ensc/metrics-ccache-no.xml.gz

(--> nearly xml files; each of them contains three <build/> sections
where the first two are about some preparation tasks and the last one is
the real build).

Results:
                               ccache        no-ccache
  total build time              5419s             5049s
  stime                         2463s             2341s
  utime                         9326s             9378s
  cache hitrate              4577:42857 = 10%        -


ccache builds from sratch are indeed slower than native builds.

But (at least my) real work does not create the image from scratch but
rebuilds existing packages or compiles own source.  As a very ideal
example, 'do_compile' of kernel needs

  total build time                63s             305s
  stime                           12s              38s
  utime                           48s             305s
  cache hitrate             nearly 100%[2]



Enrico

Footnotes: 
[1]  https://www.cvg.de/people/ensc/elito-metrics.bbclass
     https://www.cvg.de/people/ensc/metrics.py

[2]  can not be determined directly; rebuilding whole kernel shows 3
     cache misses vs. 1755 hits

Patch

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 1f76023..9f9c803 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -309,6 +309,14 @@  do_build () {
 python () {
     import exceptions, string, re
 
+    # Disable ccache explicitly if CCACHE is null since gcc may be a
+    # symlink of ccache some distributions (e.g., Fedora 17). Please
+    # note that only the existence of $CCACHE_DISABLE matters; the
+    # value is ignored.
+    if d.getVar('CCACHE', True) == '':
+        d.setVar('CCACHE_DISABLE', True)
+        d.setVarFlag('CCACHE_DISABLE', 'export', True)
+
     # Handle PACKAGECONFIG
     #
     # These take the form:
diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index a8a1665..1bb66ad 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -420,9 +420,6 @@  export PATH
 ##################################################################
 
 CCACHE ??= ""
-# Disable ccache explicitly if CCACHE is null since gcc may be a symlink
-# of ccache some distributions (e.g., Fedora 17).
-export CCACHE_DISABLE ??= "${@[0,1][d.getVar('CCACHE', True) == '']}"
 # Assign CCACHE_DIR a default value to fix a bug of ccache 3.1.7,
 # since it would always create CCACHE_DIR/.ccache even if
 # CCACHE_DISABLE = 1.