Patchwork lib/oe/path: Fix performance issue got copyhardlinktree()

login
register
mail settings
Submitter Richard Purdie
Date Nov. 8, 2013, 3:19 p.m.
Message ID <1383923945.2345.5.camel@ted>
Download mbox | patch
Permalink /patch/61333/
State New
Headers show

Comments

Richard Purdie - Nov. 8, 2013, 3:19 p.m.
With the directory copy was added to avoid race issues, it wasn't noticed that
tar was recursing the directories and copying files too. This is completely
crazy when we hardlink those files in the next command.

Resolve the issue by telling tar not to recurse. This gives a significant
performance boost to various parts of the system (do_package for linux-yocto
256s -> 178s for example).

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
Bruce Ashfield - Nov. 8, 2013, 3:42 p.m.
On 13-11-08 10:19 AM, Richard Purdie wrote:
> With the directory copy was added to avoid race issues, it wasn't noticed that
> tar was recursing the directories and copying files too. This is completely
> crazy when we hardlink those files in the next command.
>
> Resolve the issue by telling tar not to recurse. This gives a significant
> performance boost to various parts of the system (do_package for linux-yocto
> 256s -> 178s for example).

Every second makes a difference in this beast .. when adding up a
zillion package runs a week. :)

Cheers,

Bruce

>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
> diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
> index 1310e38..d0588ba 100644
> --- a/meta/lib/oe/path.py
> +++ b/meta/lib/oe/path.py
> @@ -93,7 +93,7 @@ def copyhardlinktree(src, dst):
>       if (os.stat(src).st_dev ==  os.stat(dst).st_dev):
>           # Need to copy directories only with tar first since cp will error if two
>           # writers try and create a directory at the same time
> -        cmd = 'cd %s; find . -type d -print | tar -cf - -C %s -p --files-from - | tar -xf - -C %s' % (src, src, dst)
> +        cmd = 'cd %s; find . -type d -print | tar -cf - -C %s -p --files-from - --no-recursion | tar -xf - -C %s' % (src, src, dst)
>           check_output(cmd, shell=True, stderr=subprocess.STDOUT)
>           if os.path.isdir(src):
>               src = src + "/*"
>
>
Andrea Adami - Nov. 9, 2013, 8:43 p.m.
On Fri, Nov 8, 2013 at 4:42 PM, Bruce Ashfield
<bruce.ashfield@windriver.com> wrote:
> On 13-11-08 10:19 AM, Richard Purdie wrote:
>>
>> With the directory copy was added to avoid race issues, it wasn't noticed
>> that
>> tar was recursing the directories and copying files too. This is
>> completely
>> crazy when we hardlink those files in the next command.
>>
>> Resolve the issue by telling tar not to recurse. This gives a significant
>> performance boost to various parts of the system (do_package for
>> linux-yocto
>> 256s -> 178s for example).
>
>
> Every second makes a difference in this beast .. when adding up a
> zillion package runs a week. :)
>
> Cheers,
>
> Bruce
>
>
>>
>> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>> ---
>> diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
>> index 1310e38..d0588ba 100644
>> --- a/meta/lib/oe/path.py
>> +++ b/meta/lib/oe/path.py
>> @@ -93,7 +93,7 @@ def copyhardlinktree(src, dst):
>>       if (os.stat(src).st_dev ==  os.stat(dst).st_dev):
>>           # Need to copy directories only with tar first since cp will
>> error if two
>>           # writers try and create a directory at the same time
>> -        cmd = 'cd %s; find . -type d -print | tar -cf - -C %s -p
>> --files-from - | tar -xf - -C %s' % (src, src, dst)
>> +        cmd = 'cd %s; find . -type d -print | tar -cf - -C %s -p
>> --files-from - --no-recursion | tar -xf - -C %s' % (src, src, dst)
>>           check_output(cmd, shell=True, stderr=subprocess.STDOUT)
>>           if os.path.isdir(src):
>>               src = src + "/*"
>>
>>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

After this patch do_populate_sysroot fails:

ERROR: Function failed: kernelscripts_sstate_postinst (log file is
located at /oe/oe-core/build/tmp-eglibc/work/collie-oe-linux-gnueabi/linux-yocto/3.10.17+gitAUTOINC+f1c9080cd2_c03195ed6e-r0/temp/log.do_populate_sysroot.26363)
ERROR: Logfile of failure stored in:
/oe/oe-core/build/tmp-eglibc/work/collie-oe-linux-gnueabi/linux-yocto/3.10.17+gitAUTOINC+f1c9080cd2_c03195ed6e-r0/temp/log.do_populate_sysroot.26363
Log data follows:
| DEBUG: Executing python function sstate_task_prefunc
| DEBUG: Python function sstate_task_prefunc finished
| DEBUG: Executing python function do_populate_sysroot
| DEBUG: Executing python function sysroot_stage_all
| DEBUG: Python function sysroot_stage_all finished
| DEBUG: Python function do_populate_sysroot finished
| DEBUG: Executing python function do_qa_staging
| NOTE: QA checking staging
| DEBUG: Python function do_qa_staging finished
| DEBUG: Executing python function sstate_task_postfunc
| DEBUG: Staging files from
/oe/oe-core/build/tmp-eglibc/work/collie-oe-linux-gnueabi/linux-yocto/3.10.17+gitAUTOINC+f1c9080cd2_c03195ed6e-r0/sysroot-destdir
to /oe/oe-core/build/tmp-eglibc/sysroots/collie
| DEBUG: Executing shell function kernelscripts_sstate_postinst
| NOTE: make scripts
|   HOSTCC  scripts/basic/fixdep
|   HOSTCC  scripts/kconfig/conf.o
|   SHIPPED scripts/kconfig/zconf.tab.c
|   SHIPPED scripts/kconfig/zconf.lex.c
|   SHIPPED scripts/kconfig/zconf.hash.c
|   HOSTCC  scripts/kconfig/zconf.tab.o
|   HOSTLD  scripts/kconfig/conf
| scripts/kconfig/conf --silentoldconfig Kconfig
| ***
| *** Configuration file ".config" not found!
| ***
| *** Please run some configurator (e.g. "make oldconfig" or
| *** "make menuconfig" or "make xconfig").
| ***
| make[2]: *** [silentoldconfig] Error 1
| make[1]: *** [silentoldconfig] Error 2
| make: *** No rule to make target `include/config/auto.conf', needed
by `scripts'.  Stop.
| ERROR: oe_runmake failed
| WARNING: /oe/oe-core/build/tmp-eglibc/work/collie-oe-linux-gnueabi/linux-yocto/3.10.17+gitAUTOINC+f1c9080cd2_c03195ed6e-r0/temp/run.kernelscripts_sstate_postinst.26363:1
exit 1 from
|   [ "populate_sysroot" = "populate_sysroot" -o "populate_sysroot" =
"populate_sysroot_setscene" ]
| DEBUG: Python function sstate_task_postfunc finished
| ERROR: Function failed: kernelscripts_sstate_postinst (log file is
located at /oe/oe-core/build/tmp-eglibc/work/collie-oe-linux-gnueabi/linux-yocto/3.10.17+gitAUTOINC+f1c9080cd2_c03195ed6e-r0/temp/log.do_populate_sysroot.26363)
ERROR: Task 5 (/oe/oe-core/meta/recipes-kernel/linux/linux-yocto_3.10.bb,
do_populate_sysroot) failed with exit code '1'


Cheers

Andrea
Richard Purdie - Nov. 9, 2013, 10:52 p.m.
On Sat, 2013-11-09 at 21:43 +0100, Andrea Adami wrote:
> On Fri, Nov 8, 2013 at 4:42 PM, Bruce Ashfield
> <bruce.ashfield@windriver.com> wrote:
> > On 13-11-08 10:19 AM, Richard Purdie wrote:
> >>
> >> With the directory copy was added to avoid race issues, it wasn't noticed
> >> that
> >> tar was recursing the directories and copying files too. This is
> >> completely
> >> crazy when we hardlink those files in the next command.
> >>
> >> Resolve the issue by telling tar not to recurse. This gives a significant
> >> performance boost to various parts of the system (do_package for
> >> linux-yocto
> >> 256s -> 178s for example).
> >
> >
> > Every second makes a difference in this beast .. when adding up a
> > zillion package runs a week. :)
> >
> > Cheers,
> >
> > Bruce
> >
> >
> >>
> >> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> >> ---
> >> diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
> >> index 1310e38..d0588ba 100644
> >> --- a/meta/lib/oe/path.py
> >> +++ b/meta/lib/oe/path.py
> >> @@ -93,7 +93,7 @@ def copyhardlinktree(src, dst):
> >>       if (os.stat(src).st_dev ==  os.stat(dst).st_dev):
> >>           # Need to copy directories only with tar first since cp will
> >> error if two
> >>           # writers try and create a directory at the same time
> >> -        cmd = 'cd %s; find . -type d -print | tar -cf - -C %s -p
> >> --files-from - | tar -xf - -C %s' % (src, src, dst)
> >> +        cmd = 'cd %s; find . -type d -print | tar -cf - -C %s -p
> >> --files-from - --no-recursion | tar -xf - -C %s' % (src, src, dst)
> >>           check_output(cmd, shell=True, stderr=subprocess.STDOUT)
> >>           if os.path.isdir(src):
> >>               src = src + "/*"
> >>
> >>
> >
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
> 
> After this patch do_populate_sysroot fails:

This should fix it:

http://git.yoctoproject.org/cgit.cgi/poky-contrib/commit/?h=rpurdie/t2&id=5237c8e2c1780983c000247ffa0c890b4bb5980a

Cheers,

Richard
Andrea Adami - Nov. 9, 2013, 11:58 p.m.
On Sat, Nov 9, 2013 at 11:52 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Sat, 2013-11-09 at 21:43 +0100, Andrea Adami wrote:
>> On Fri, Nov 8, 2013 at 4:42 PM, Bruce Ashfield
>> <bruce.ashfield@windriver.com> wrote:
>> > On 13-11-08 10:19 AM, Richard Purdie wrote:
>> >>
>> >> With the directory copy was added to avoid race issues, it wasn't noticed
>> >> that
>> >> tar was recursing the directories and copying files too. This is
>> >> completely
>> >> crazy when we hardlink those files in the next command.
>> >>
>> >> Resolve the issue by telling tar not to recurse. This gives a significant
>> >> performance boost to various parts of the system (do_package for
>> >> linux-yocto
>> >> 256s -> 178s for example).
>> >
>> >
>> > Every second makes a difference in this beast .. when adding up a
>> > zillion package runs a week. :)
>> >
>> > Cheers,
>> >
>> > Bruce
>> >
>> >
>> >>
>> >> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>> >> ---
>> >> diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
>> >> index 1310e38..d0588ba 100644
>> >> --- a/meta/lib/oe/path.py
>> >> +++ b/meta/lib/oe/path.py
>> >> @@ -93,7 +93,7 @@ def copyhardlinktree(src, dst):
>> >>       if (os.stat(src).st_dev ==  os.stat(dst).st_dev):
>> >>           # Need to copy directories only with tar first since cp will
>> >> error if two
>> >>           # writers try and create a directory at the same time
>> >> -        cmd = 'cd %s; find . -type d -print | tar -cf - -C %s -p
>> >> --files-from - | tar -xf - -C %s' % (src, src, dst)
>> >> +        cmd = 'cd %s; find . -type d -print | tar -cf - -C %s -p
>> >> --files-from - --no-recursion | tar -xf - -C %s' % (src, src, dst)
>> >>           check_output(cmd, shell=True, stderr=subprocess.STDOUT)
>> >>           if os.path.isdir(src):
>> >>               src = src + "/*"
>> >>
>> >>
>> >
>> > _______________________________________________
>> > Openembedded-core mailing list
>> > Openembedded-core@lists.openembedded.org
>> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
>>
>> After this patch do_populate_sysroot fails:
>
> This should fix it:
>
> http://git.yoctoproject.org/cgit.cgi/poky-contrib/commit/?h=rpurdie/t2&id=5237c8e2c1780983c000247ffa0c890b4bb5980a
>
> Cheers,
>
> Richard
>

Yes, thx for the magic fix.
Now with all the last patches my results are much better:

do_install:
Elapsed time: 83.77 seconds CPU usage: 3.0% (worst before 131s)
(avg before:103 seconds CPU usage: 2.9%)

do_populate_sysroot:
Elapsed time: 16.12 seconds CPU usage: 18.5%
(avg before: 95 seconds CPU usage: 6.5%)

Cheers

Andrea

Patch

diff --git a/meta/lib/oe/path.py b/meta/lib/oe/path.py
index 1310e38..d0588ba 100644
--- a/meta/lib/oe/path.py
+++ b/meta/lib/oe/path.py
@@ -93,7 +93,7 @@  def copyhardlinktree(src, dst):
     if (os.stat(src).st_dev ==  os.stat(dst).st_dev):
         # Need to copy directories only with tar first since cp will error if two 
         # writers try and create a directory at the same time
-        cmd = 'cd %s; find . -type d -print | tar -cf - -C %s -p --files-from - | tar -xf - -C %s' % (src, src, dst)
+        cmd = 'cd %s; find . -type d -print | tar -cf - -C %s -p --files-from - --no-recursion | tar -xf - -C %s' % (src, src, dst)
         check_output(cmd, shell=True, stderr=subprocess.STDOUT)
         if os.path.isdir(src):
             src = src + "/*"