mbox

Lucid build performance improvements pull request

Message ID 4E5416DC.90205@canonical.com
State New
Headers show

Pull-request

git://kernel.ubuntu.com/rtg/ubuntu-lucid.git makefile-improvenets

Message

Tim Gardner Aug. 23, 2011, 9:08 p.m. UTC
These patches in Oneiric seem to be working pretty well. With these 
changes and sufficient server capability I've seen build times cut in 
half. Now you'll be able to control concurrency from the command line, e.g.,

dpkg-buildpackage -j`getconf _NPROCESSORS_ONLN` -B

http://bugs.launchpad.net/bugs/832352

The following changes since commit 2f28aed962765c698f9ed3bc2385dbe79ada2514:
   Seth Forshee (1):
         drm/i915: Remove BUG_ON from i915_gem_evict_something

are available in the git repository at:

   git://kernel.ubuntu.com/rtg/ubuntu-lucid.git makefile-improvenets

Tim Gardner (11):
       UBUNTU: [Config] Simplify binary-udebs dependencies
       UBUNTU: [Config] kernel preparation cannot be parallelized
       UBUNTU: [Config] Linearize module/abi checks
       UBUNTU: [Config] Linearize and simplify tree preparation rules
       UBUNTU: [Config] Build kernel image in parallel with modules
       UBUNTU: [Config] Set concurrency for kmake invocations
       UBUNTU: [Config] Improve install-arch-headers speed
       UBUNTU: [Config] Fix binary-perarch dependencies
       UBUNTU: [Config] Removed stamp-flavours target
       UBUNTU: [Config] Serialize binary indep targets
       UBUNTU: [Config] Use build stamp directly

  debian/rules.d/2-binary-arch.mk  |   35 
++++++++++++++---------------------
  debian/rules.d/3-binary-indep.mk |   29 ++++++++++++-----------------
  debian/rules.d/4-checks.mk       |   10 ++--------
  debian/rules.d/5-udebs.mk        |    4 ++--
  4 files changed, 30 insertions(+), 48 deletions(-)

Comments

Herton Ronaldo Krzesinski Aug. 25, 2011, 8:36 p.m. UTC | #1
On Tue, Aug 23, 2011 at 03:08:44PM -0600, Tim Gardner wrote:
> These patches in Oneiric seem to be working pretty well. With these
> changes and sufficient server capability I've seen build times cut
> in half. Now you'll be able to control concurrency from the command
> line, e.g.,
> 
> dpkg-buildpackage -j`getconf _NPROCESSORS_ONLN` -B
> 
> http://bugs.launchpad.net/bugs/832352
> 
> The following changes since commit 2f28aed962765c698f9ed3bc2385dbe79ada2514:
>   Seth Forshee (1):
>         drm/i915: Remove BUG_ON from i915_gem_evict_something
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/rtg/ubuntu-lucid.git makefile-improvenets
> 
> Tim Gardner (11):
>       UBUNTU: [Config] Simplify binary-udebs dependencies
>       UBUNTU: [Config] kernel preparation cannot be parallelized
>       UBUNTU: [Config] Linearize module/abi checks
>       UBUNTU: [Config] Linearize and simplify tree preparation rules
>       UBUNTU: [Config] Build kernel image in parallel with modules
>       UBUNTU: [Config] Set concurrency for kmake invocations
>       UBUNTU: [Config] Improve install-arch-headers speed
>       UBUNTU: [Config] Fix binary-perarch dependencies
>       UBUNTU: [Config] Removed stamp-flavours target
>       UBUNTU: [Config] Serialize binary indep targets
>       UBUNTU: [Config] Use build stamp directly

I'm no make expert, but I noted some things.

In changes

UBUNTU: [Config] kernel preparation cannot be parallelized
UBUNTU: [Config] Set concurrency for kmake invocations

you force -j1. Indeed scripts target depend on oldconfig/prepare first:

$(build_cd) $(kmake) $(build_O) -j1 silentoldconfig prepare scripts

but each target could benefit from parallelism (which I expect to be the
case of scripts target), so a better invocation would be:

$(build_cd) $(kmake) $(build_O) silentoldconfig
$(build_cd) $(kmake) $(build_O) prepare
$(build_cd) $(kmake) $(build_O) scripts

Also, prepare alread implies/runs silentoldconfig, so we could simplify
even more I think:

$(build_cd) $(kmake) $(build_O) prepare
$(build_cd) $(kmake) $(build_O) scripts

I don't expect significant improvement, but would be better.

In change

UBUNTU: [Config] Linearize module/abi checks

you could remove the install -d below "+$(abidir)/%.modules: abi-check-%"
since through abi-check-% -> $(abidir)/% we already get the directory 
created.

Also, on a quick look, seems this change is unecessary:
abi-check script doesn't seem change/care about the modules list
module-check script also only cares about modules list
Unless I'm missing something else.


In change

UBUNTU: [Config] Serialize binary indep targets

I'm in doubt, why serialize it?


The rest looks ok to me, it's great having a better build time.

> 
>  debian/rules.d/2-binary-arch.mk  |   35
> ++++++++++++++---------------------
>  debian/rules.d/3-binary-indep.mk |   29 ++++++++++++-----------------
>  debian/rules.d/4-checks.mk       |   10 ++--------
>  debian/rules.d/5-udebs.mk        |    4 ++--
>  4 files changed, 30 insertions(+), 48 deletions(-)
> -- 
> Tim Gardner tim.gardner@canonical.com
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
Herton Ronaldo Krzesinski Aug. 25, 2011, 8:43 p.m. UTC | #2
On Thu, Aug 25, 2011 at 05:36:31PM -0300, Herton Ronaldo Krzesinski wrote:
> On Tue, Aug 23, 2011 at 03:08:44PM -0600, Tim Gardner wrote:
> > These patches in Oneiric seem to be working pretty well. With these
> > changes and sufficient server capability I've seen build times cut
> > in half. Now you'll be able to control concurrency from the command
> > line, e.g.,
> > 
> > dpkg-buildpackage -j`getconf _NPROCESSORS_ONLN` -B
> > 
> > http://bugs.launchpad.net/bugs/832352
> > 
> > The following changes since commit 2f28aed962765c698f9ed3bc2385dbe79ada2514:
> >   Seth Forshee (1):
> >         drm/i915: Remove BUG_ON from i915_gem_evict_something
> > 
> > are available in the git repository at:
> > 
> >   git://kernel.ubuntu.com/rtg/ubuntu-lucid.git makefile-improvenets
> > 
> > Tim Gardner (11):
> >       UBUNTU: [Config] Simplify binary-udebs dependencies
> >       UBUNTU: [Config] kernel preparation cannot be parallelized
> >       UBUNTU: [Config] Linearize module/abi checks
> >       UBUNTU: [Config] Linearize and simplify tree preparation rules
> >       UBUNTU: [Config] Build kernel image in parallel with modules
> >       UBUNTU: [Config] Set concurrency for kmake invocations
> >       UBUNTU: [Config] Improve install-arch-headers speed
> >       UBUNTU: [Config] Fix binary-perarch dependencies
> >       UBUNTU: [Config] Removed stamp-flavours target
> >       UBUNTU: [Config] Serialize binary indep targets
> >       UBUNTU: [Config] Use build stamp directly
> 
> I'm no make expert, but I noted some things.
> 
> In changes
> 
> UBUNTU: [Config] kernel preparation cannot be parallelized
> UBUNTU: [Config] Set concurrency for kmake invocations
> 
> you force -j1. Indeed scripts target depend on oldconfig/prepare first:
> 
> $(build_cd) $(kmake) $(build_O) -j1 silentoldconfig prepare scripts
> 
> but each target could benefit from parallelism (which I expect to be the
> case of scripts target), so a better invocation would be:
> 
> $(build_cd) $(kmake) $(build_O) silentoldconfig
> $(build_cd) $(kmake) $(build_O) prepare
> $(build_cd) $(kmake) $(build_O) scripts
> 
> Also, prepare alread implies/runs silentoldconfig, so we could simplify
> even more I think:
> 
> $(build_cd) $(kmake) $(build_O) prepare
> $(build_cd) $(kmake) $(build_O) scripts

I missed $(conc_level), should be:
$(build_cd) $(kmake) $(build_O) $(conc_level) prepare
$(build_cd) $(kmake) $(build_O) $(conc_level) scripts

> 
> I don't expect significant improvement, but would be better.
> 
> In change
> 
> UBUNTU: [Config] Linearize module/abi checks
> 
> you could remove the install -d below "+$(abidir)/%.modules: abi-check-%"
> since through abi-check-% -> $(abidir)/% we already get the directory 
> created.
> 
> Also, on a quick look, seems this change is unecessary:
> abi-check script doesn't seem change/care about the modules list
> module-check script also only cares about modules list
> Unless I'm missing something else.
> 
> 
> In change
> 
> UBUNTU: [Config] Serialize binary indep targets
> 
> I'm in doubt, why serialize it?
> 
> 
> The rest looks ok to me, it's great having a better build time.
> 
> > 
> >  debian/rules.d/2-binary-arch.mk  |   35
> > ++++++++++++++---------------------
> >  debian/rules.d/3-binary-indep.mk |   29 ++++++++++++-----------------
> >  debian/rules.d/4-checks.mk       |   10 ++--------
> >  debian/rules.d/5-udebs.mk        |    4 ++--
> >  4 files changed, 30 insertions(+), 48 deletions(-)
> > -- 
> > Tim Gardner tim.gardner@canonical.com
> > 
> > -- 
> > kernel-team mailing list
> > kernel-team@lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team
> > 
> 
> -- 
> []'s
> Herton
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
Stefan Bader Aug. 26, 2011, 10:15 a.m. UTC | #3
On 23.08.2011 23:08, Tim Gardner wrote:
> These patches in Oneiric seem to be working pretty well. With these changes and
> sufficient server capability I've seen build times cut in half. Now you'll be
> able to control concurrency from the command line, e.g.,
> 
> dpkg-buildpackage -j`getconf _NPROCESSORS_ONLN` -B
> 
> http://bugs.launchpad.net/bugs/832352
> 
> The following changes since commit 2f28aed962765c698f9ed3bc2385dbe79ada2514:
>   Seth Forshee (1):
>         drm/i915: Remove BUG_ON from i915_gem_evict_something
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/rtg/ubuntu-lucid.git makefile-improvenets
> 
> Tim Gardner (11):
>       UBUNTU: [Config] Simplify binary-udebs dependencies
>       UBUNTU: [Config] kernel preparation cannot be parallelized
>       UBUNTU: [Config] Linearize module/abi checks
>       UBUNTU: [Config] Linearize and simplify tree preparation rules
>       UBUNTU: [Config] Build kernel image in parallel with modules
>       UBUNTU: [Config] Set concurrency for kmake invocations
>       UBUNTU: [Config] Improve install-arch-headers speed
>       UBUNTU: [Config] Fix binary-perarch dependencies
>       UBUNTU: [Config] Removed stamp-flavours target
>       UBUNTU: [Config] Serialize binary indep targets
>       UBUNTU: [Config] Use build stamp directly
> 
>  debian/rules.d/2-binary-arch.mk  |   35 ++++++++++++++---------------------
>  debian/rules.d/3-binary-indep.mk |   29 ++++++++++++-----------------
>  debian/rules.d/4-checks.mk       |   10 ++--------
>  debian/rules.d/5-udebs.mk        |    4 ++--
>  4 files changed, 30 insertions(+), 48 deletions(-)

As much as I would like improved build times, the voice on the other side tells
me it may be just a good idea to let changes/improvements settle a bit in
Oneiric before pulling things all the way back... But then I am usually at the
airport 2 hrs ahead of time...

-Stefan
Tim Gardner Aug. 26, 2011, 1:01 p.m. UTC | #4
On 08/25/2011 02:36 PM, Herton Ronaldo Krzesinski wrote:
> On Tue, Aug 23, 2011 at 03:08:44PM -0600, Tim Gardner wrote:
>> These patches in Oneiric seem to be working pretty well. With these
>> changes and sufficient server capability I've seen build times cut
>> in half. Now you'll be able to control concurrency from the command
>> line, e.g.,
>>
>> dpkg-buildpackage -j`getconf _NPROCESSORS_ONLN` -B
>>
>> http://bugs.launchpad.net/bugs/832352
>>
>> The following changes since commit 2f28aed962765c698f9ed3bc2385dbe79ada2514:
>>    Seth Forshee (1):
>>          drm/i915: Remove BUG_ON from i915_gem_evict_something
>>
>> are available in the git repository at:
>>
>>    git://kernel.ubuntu.com/rtg/ubuntu-lucid.git makefile-improvenets
>>
>> Tim Gardner (11):
>>        UBUNTU: [Config] Simplify binary-udebs dependencies
>>        UBUNTU: [Config] kernel preparation cannot be parallelized
>>        UBUNTU: [Config] Linearize module/abi checks
>>        UBUNTU: [Config] Linearize and simplify tree preparation rules
>>        UBUNTU: [Config] Build kernel image in parallel with modules
>>        UBUNTU: [Config] Set concurrency for kmake invocations
>>        UBUNTU: [Config] Improve install-arch-headers speed
>>        UBUNTU: [Config] Fix binary-perarch dependencies
>>        UBUNTU: [Config] Removed stamp-flavours target
>>        UBUNTU: [Config] Serialize binary indep targets
>>        UBUNTU: [Config] Use build stamp directly
>
> I'm no make expert, but I noted some things.
>

My goals with these changes are to make the minimum number of changes 
whilst simplifying the rules and enabling parallel building where it 
makes sense.

> In changes
>
> UBUNTU: [Config] kernel preparation cannot be parallelized
> UBUNTU: [Config] Set concurrency for kmake invocations
>
> you force -j1. Indeed scripts target depend on oldconfig/prepare first:
>
> $(build_cd) $(kmake) $(build_O) -j1 silentoldconfig prepare scripts
>
> but each target could benefit from parallelism (which I expect to be the
> case of scripts target), so a better invocation would be:
>
> $(build_cd) $(kmake) $(build_O) silentoldconfig
> $(build_cd) $(kmake) $(build_O) prepare
> $(build_cd) $(kmake) $(build_O) scripts
>
> Also, prepare alread implies/runs silentoldconfig, so we could simplify
> even more I think:
>
> $(build_cd) $(kmake) $(build_O) prepare
> $(build_cd) $(kmake) $(build_O) scripts
>
> I don't expect significant improvement, but would be better.
>

In the interest of not over complicating things I chose to leave this 
rule alone. It runs fast enough that its not worth optimizing.

> In change
>
> UBUNTU: [Config] Linearize module/abi checks
>
> you could remove the install -d below "+$(abidir)/%.modules: abi-check-%"
> since through abi-check-% ->  $(abidir)/% we already get the directory
> created.
>
> Also, on a quick look, seems this change is unecessary:
> abi-check script doesn't seem change/care about the modules list
> module-check script also only cares about modules list
> Unless I'm missing something else.
>
>
> In change
>
> UBUNTU: [Config] Serialize binary indep targets
>
> I'm in doubt, why serialize it?
>

While none of the indep packages are dependent, creating them linearly 
(since they are quite small) makes the rule set very straightforward. I 
don't think there is much to be gained with an independent rule set here.

>
> The rest looks ok to me, it's great having a better build time.
>
>>
>>   debian/rules.d/2-binary-arch.mk  |   35
>> ++++++++++++++---------------------
>>   debian/rules.d/3-binary-indep.mk |   29 ++++++++++++-----------------
>>   debian/rules.d/4-checks.mk       |   10 ++--------
>>   debian/rules.d/5-udebs.mk        |    4 ++--
>>   4 files changed, 30 insertions(+), 48 deletions(-)
>> --
>> Tim Gardner tim.gardner@canonical.com
>>
>> --
>> kernel-team mailing list
>> kernel-team@lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>>
>
Tim Gardner Aug. 31, 2011, 7:56 p.m. UTC | #5
On 08/23/2011 03:08 PM, Tim Gardner wrote:
> These patches in Oneiric seem to be working pretty well. With these
> changes and sufficient server capability I've seen build times cut in
> half. Now you'll be able to control concurrency from the command line,
> e.g.,
>
> dpkg-buildpackage -j`getconf _NPROCESSORS_ONLN` -B
>
> http://bugs.launchpad.net/bugs/832352
>
> The following changes since commit
> 2f28aed962765c698f9ed3bc2385dbe79ada2514:
> Seth Forshee (1):
> drm/i915: Remove BUG_ON from i915_gem_evict_something
>
> are available in the git repository at:
>
> git://kernel.ubuntu.com/rtg/ubuntu-lucid.git makefile-improvenets
>
> Tim Gardner (11):
> UBUNTU: [Config] Simplify binary-udebs dependencies
> UBUNTU: [Config] kernel preparation cannot be parallelized
> UBUNTU: [Config] Linearize module/abi checks
> UBUNTU: [Config] Linearize and simplify tree preparation rules
> UBUNTU: [Config] Build kernel image in parallel with modules
> UBUNTU: [Config] Set concurrency for kmake invocations
> UBUNTU: [Config] Improve install-arch-headers speed
> UBUNTU: [Config] Fix binary-perarch dependencies
> UBUNTU: [Config] Removed stamp-flavours target
> UBUNTU: [Config] Serialize binary indep targets
> UBUNTU: [Config] Use build stamp directly
>
> debian/rules.d/2-binary-arch.mk | 35 ++++++++++++++---------------------
> debian/rules.d/3-binary-indep.mk | 29 ++++++++++++-----------------
> debian/rules.d/4-checks.mk | 10 ++--------
> debian/rules.d/5-udebs.mk | 4 ++--
> 4 files changed, 30 insertions(+), 48 deletions(-)

I've done plenty of testing, so I think its time to let this cook in 
master-next for a bit in the pre-proposed PPA.

rtg