Patchwork Some further build dependency timings

login
register
mail settings
Submitter Richard Purdie
Date Oct. 11, 2011, 11:35 a.m.
Message ID <1318332940.23801.90.camel@ted>
Download mbox | patch
Permalink /patch/13005/
State New, archived
Headers show

Comments

Richard Purdie - Oct. 11, 2011, 11:35 a.m.
On Mon, 2011-10-10 at 21:45 +0100, Richard Purdie wrote:
> Just for reference, with a base configuration, sato image:
> 
> real	50m8.223s
> user	298m41.450s
> sys	52m42.200s
> 
> adding:
> 
> ASSUME_PROVIDED =+ "bison-native flex-native sqlite3-native git-native"
> 
> (and hacking the pseudo recipe to use a sqlite3-native):
> 
> real	42m6.740s
> user	296m21.940s
> sys	52m25.220s
> 
> We continue to have real dependency issues around gettext in both the
> native and target builds...

So to continue the story, adding:

DEBUG_FLAGS = ""
INHERIT_INSANE = ""
PACKAGE_CLASSES = "package_ipk"
USER_CLASSES = ""

Results in:

real	38m23.053s
user	237m7.430s
sys	39m3.720s

Then adding PSEUDO_RELOADED=yes to the environment of tasks not needing
pseudo (hack patch below):

real	36m20.683s
user	236m19.580s
sys	37m54.450s

Finally, adding:

DISABLESTATIC = "--disable-static"
DISABLESTATIC_pn-qemu = ""
DISABLESTATIC_pn-qemu-native = ""
DISABLESTATIC_pn-openssl = ""
DISABLESTATIC_pn-openssl-native = ""
EXTRA_OECONF += "${DISABLESTATIC}"

real	34m53.877s
user	233m34.780s
sys	38m50.190s

Cheers,

Richard
Mark Hatle - Oct. 12, 2011, 2:40 p.m.
On 10/11/11 6:35 AM, Richard Purdie wrote:
> On Mon, 2011-10-10 at 21:45 +0100, Richard Purdie wrote:
>> Just for reference, with a base configuration, sato image:
>>
>> real	50m8.223s
>> user	298m41.450s
>> sys	52m42.200s
>>
>> adding:
>>
>> ASSUME_PROVIDED =+ "bison-native flex-native sqlite3-native git-native"
>>
>> (and hacking the pseudo recipe to use a sqlite3-native):
>>
>> real	42m6.740s
>> user	296m21.940s
>> sys	52m25.220s
>>
>> We continue to have real dependency issues around gettext in both the
>> native and target builds...
> 
> So to continue the story, adding:
> 
> DEBUG_FLAGS = ""
> INHERIT_INSANE = ""
> PACKAGE_CLASSES = "package_ipk"
> USER_CLASSES = ""
> 
> Results in:
> 
> real	38m23.053s
> user	237m7.430s
> sys	39m3.720s
> 
> Then adding PSEUDO_RELOADED=yes to the environment of tasks not needing
> pseudo (hack patch below):
> 
> real	36m20.683s
> user	236m19.580s
> sys	37m54.450s
> 
> Finally, adding:
> 
> DISABLESTATIC = "--disable-static"
> DISABLESTATIC_pn-qemu = ""
> DISABLESTATIC_pn-qemu-native = ""
> DISABLESTATIC_pn-openssl = ""
> DISABLESTATIC_pn-openssl-native = ""
> EXTRA_OECONF += "${DISABLESTATIC}"
> 
> real	34m53.877s
> user	233m34.780s
> sys	38m50.190s
> 
> Cheers,
> 
> Richard
> 
> 
> --- a/bitbake/lib/bb/runqueue.py
> +++ b/bitbake/lib/bb/runqueue.py
> @@ -1096,6 +1096,10 @@ class RunQueueExecute:
>  
>              logger.debug(2, 'Running %s:%s under fakeroot, fakedirs: %s' %
>                              (fn, taskname, ', '.join(fakedirs)))
> +        else:
> +            envbackup["PSEUDO_RELOADED"] = os.environ.get("PSEUDO_RELOADED")
> +            os.environ["PSEUDO_RELOADED"] = "yes"
> +            fakeenv["PSEUDO_RELOADED"] = "yes"

PSEUDO_RELOADED was intended to only be used internally by pseudo.  Specifically
to signify to fork/exec that we're about to run the pseudo server process and to
remove PSEUDO from the LD_PRELOAD on the next fork/exec.

The system simply checks the environment on the fork-exec and doesn't check the
value on pseudo-load.  This is why it works.

That side effect means that any subsequent calls can NOT re-enable pseudo as
it's no longer in memory at all.

So what you are seeing at best is the time difference from avoiding the
LD_PRELOAD and the jump table setup.  (Even when pseudo is disabled, we need to
enable the jump table, we just pass the call to the original function.)

But more then likely because pseudo is no longer memory resident for the called
processes and it's sub-processes.. there is likely some place in the system that
uses pseudo functionality that is no longer functioning properly.

It would be interesting to determine first off, if there is anywhere pseudo
should be running that it currently isn't.  And second if this is simply due to
LD_PRELOAD time, or if enabling the jump table is the culprit.

One potential optimization, when PSEUDO_DISABLE=1 is to only setup the jump
table entries for fork/exec items.

Also if it is desired to unload pseudo, I'd suggest we add a new environment
variable to pseudo that is specific about unloading pseudo, instead of using the
existing PSEUDO_RELOADED.  Not only is the name confusing, but since it's an
internal item it's behavior may change or go away in the future.

(Below is from the pseudo man page:

      PSEUDO_RELOADED
               This purely internal variable is used to track state while try-
               ing to re-execute to get  rid  of  the  LD_PRELOAD  value  when
               spawning  a  server.  (The pseudo server itself cannot function
               running in the pseudo environment.)
)

>  
>          sys.stdout.flush()
>          sys.stderr.flush()
> 
> 
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Richard Purdie - Oct. 12, 2011, 2:49 p.m.
On Wed, 2011-10-12 at 09:40 -0500, Mark Hatle wrote:
> > --- a/bitbake/lib/bb/runqueue.py
> > +++ b/bitbake/lib/bb/runqueue.py
> > @@ -1096,6 +1096,10 @@ class RunQueueExecute:
> >  
> >              logger.debug(2, 'Running %s:%s under fakeroot, fakedirs: %s' %
> >                              (fn, taskname, ', '.join(fakedirs)))
> > +        else:
> > +            envbackup["PSEUDO_RELOADED"] = os.environ.get("PSEUDO_RELOADED")
> > +            os.environ["PSEUDO_RELOADED"] = "yes"
> > +            fakeenv["PSEUDO_RELOADED"] = "yes"
> 
> PSEUDO_RELOADED was intended to only be used internally by pseudo.  Specifically
> to signify to fork/exec that we're about to run the pseudo server process and to
> remove PSEUDO from the LD_PRELOAD on the next fork/exec.

Right, I realised I was abusing it here, its good for a performance
measurement though ;-)

> The system simply checks the environment on the fork-exec and doesn't check the
> value on pseudo-load.  This is why it works.
> 
> That side effect means that any subsequent calls can NOT re-enable pseudo as
> it's no longer in memory at all.
>
> So what you are seeing at best is the time difference from avoiding the
> LD_PRELOAD and the jump table setup.  (Even when pseudo is disabled, we need to
> enable the jump table, we just pass the call to the original function.)
> 
> But more then likely because pseudo is no longer memory resident for the called
> processes and it's sub-processes.. there is likely some place in the system that
> uses pseudo functionality that is no longer functioning properly.

I'm not sure that there is in our use cases. If we need pseudo
functionality the tasks are marked as "fakeroot" with a flag. If that
flag is not set (which is when the above code triggers), its perfectly
fine to have pseudo unload at the next exec.

> It would be interesting to determine first off, if there is anywhere pseudo
> should be running that it currently isn't.  And second if this is simply due to
> LD_PRELOAD time, or if enabling the jump table is the culprit.
> 
> One potential optimization, when PSEUDO_DISABLE=1 is to only setup the jump
> table entries for fork/exec items.

Agreed, I don't know where the time is being spent exactly at this
point. I do know we execute an absolute ton of exec/fork calls so
removing any overhead from them will improve our speed though. This
change means do_configure won't run with pseudo enabled for example
which is a very exec heavy task. 

> Also if it is desired to unload pseudo, I'd suggest we add a new environment
> variable to pseudo that is specific about unloading pseudo, instead of using the
> existing PSEUDO_RELOADED.  Not only is the name confusing, but since it's an
> internal item it's behavior may change or go away in the future.
> 
> (Below is from the pseudo man page:
> 
>       PSEUDO_RELOADED
>                This purely internal variable is used to track state while try-
>                ing to re-execute to get  rid  of  the  LD_PRELOAD  value  when
>                spawning  a  server.  (The pseudo server itself cannot function
>                running in the pseudo environment.)
> )

I suspect we may need a dedicated option for this...

Cheers,

Richard
Richard Purdie - Oct. 14, 2011, 11:34 a.m.
On Wed, 2011-10-12 at 15:49 +0100, Richard Purdie wrote:
> On Wed, 2011-10-12 at 09:40 -0500, Mark Hatle wrote:
> > It would be interesting to determine first off, if there is anywhere pseudo
> > should be running that it currently isn't.  And second if this is simply due to
> > LD_PRELOAD time, or if enabling the jump table is the culprit.
> > 
> > One potential optimization, when PSEUDO_DISABLE=1 is to only setup the jump
> > table entries for fork/exec items.
> 
> Agreed, I don't know where the time is being spent exactly at this
> point. I do know we execute an absolute ton of exec/fork calls so
> removing any overhead from them will improve our speed though. This
> change means do_configure won't run with pseudo enabled for example
> which is a very exec heavy task. 

So we did some tests, I thought I'd share the results:

base build config:

real	38m12.356s
user	236m4.710s
sys	39m38.180s

adding in the psuedo patches but not enabling (effectively jitter
measurement):

real	38m13.034s
user	235m44.250s
sys	39m6.150s

enabling the lazy linking:

real	38m16.121s
user	236m1.300s
sys	39m45.860s

Enable PSEUDO_UNLOAD:

real	35m46.281s
user	233m56.750s
sys	37m51.400s

So the big win isn't the linking, its the unloading of pseudo when we
don't need it...

Cheers,

Richard

Patch

--- a/bitbake/lib/bb/runqueue.py
+++ b/bitbake/lib/bb/runqueue.py
@@ -1096,6 +1096,10 @@  class RunQueueExecute:
 
             logger.debug(2, 'Running %s:%s under fakeroot, fakedirs: %s' %
                             (fn, taskname, ', '.join(fakedirs)))
+        else:
+            envbackup["PSEUDO_RELOADED"] = os.environ.get("PSEUDO_RELOADED")
+            os.environ["PSEUDO_RELOADED"] = "yes"
+            fakeenv["PSEUDO_RELOADED"] = "yes"
 
         sys.stdout.flush()
         sys.stderr.flush()