[U-Boot] [PATCH] buildman: allow more incremental building

[Simon Glass]

Hi Stephen,

On 11 April 2016 at 10:15, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 04/11/2016 06:04 AM, Simon Glass wrote:
>>
>> Hi,
>>
>> On 1 April 2016 at 07:35, Tom Rini <trini at konsulko.com> wrote:
>>>
>>> On Mon, Mar 28, 2016 at 04:39:35PM -0600, Stephen Warren wrote:
>>>
>>>> From: Stephen Warren <swarren at nvidia.com>
>>>>
>>>> One use-case for buildman is to continually run it interactively after
>>>> each small step in a large refactoring operation. This gives more
>>>> immediate feedback than making a number of commits and then going back
>>>> and
>>>> testing them. For this to work well, buildman needs to be extremely
>>>> fast.
>>>> At present, a couple issues prevent it being as fast as it could be:
>>>>
>>>> 1) Each time buildman runs "make %_defconfig", it runs "make mrproper"
>>>> first. This throws away all previous build results, requiring a
>>>> from-scratch build. Optionally avoiding this would speed up the build,
>>>> at
>>>> the cost of potentially causing or missing some build issues.
>>
>>
>> Does it actually cause problems? I think I have seen things go wrong
>> before, which is why I added it, but I can't remember the detail.
>
>
> I have not noticed any issues using -I and hence skipping the mrproper. It
> is possible you added the mrproper before the conversion to Kbuild; Kbuild
> seems to handle rebuilding after code changes much better than the old
> system, which definitely had some flaws that prevented incremental builds in
> many cases.

OK I see. For next release we should consider making -I the default.
>
>> Threads work by configuring and building the first commit for a board,
>> then incrementally building each commit for that same board. So it
>> seems like change should only be useful when all the boards you are
>> building have a similar config.
>
>
> No, because of point 2 below.
>
>>>> 2) A build tree is created per thread rather than per board. When a
>>>> thread
>>>> switches between building different boards, this often causes many files
>>>> to be rebuilt due to changing config options. Using a separate build
>>>> tree
>>>> for each board would avoid this. This does put more strain on the
>>>> system's
>>>> disk cache, but it is worth it on my system at least.
>>
>>
>> I'm not sure why this is a win, given what I said above. I'm starting
>> to feel that I don't understand what is going on.
>
>
> Since each board gets built in a separate directory, there's no build churn
> due to the configuration being changed when threads switch between building
> different boards. Right now what happens on a thread (even without mrproper)
> is:
>
> Thread 1 builds board X
> - Some stuff gets rebuilt due to source changes
> Thread 1 builds board Y
> - Some stuff gets rebuilt due to source changes
> - Lots more gets rebuilt because the config changed between the two boards,
> and bother were built in the same directory.
>
> After this patch, the following happens:
>
> Thread 1 builds board X
> - Some stuff gets rebuilt due to source changes
> Thread 1 builds board Y
> - Some stuff gets rebuilt due to source changes
> - Nothing else gets rebuilt
>
> If the source changes happen not to affect some particular board, then this
> new scheme means that nothing gets rebuilt for it, which significantly
> speeds up the build.

I understand what you are saying, but the intention of
builderthread.RunJob is to avoid this churn. It runs in a loop through
all commits, building the same board for each. What am I missing?

>
>
>>>> This commit adds two command-line options to implement the changes
>>>> described above; -I ("--incremental") turns of "make mrproper" and -P
>>>> ("--per-board-out-dir") creats a build directory per board rather than
>>>> per
>>>> thread.
>>>>
>>>> Tested:
>>>>
>>>>      ./tools/buildman/buildman.py tegra
>>>>      ./tools/buildman/buildman.py -I -P tegra
>>>>      ./tools/buildman/buildman.py -b tegra_dev tegra
>>>>      ./tools/buildman/buildman.py -b tegra_dev -I -P tegra
>>>>
>>>> ... each once after deleting the buildman result/work directory, and
>>>> once
>>>> "incrementally" after a previous identical invocation.
>>>>
>>>> Signed-off-by: Stephen Warren <swarren at nvidia.com>
>>>
>>>
>>> Reviewed-by: Tom Rini <trini at konsulko.com>
>>
>>
>> Oddly I don't seem to have received the original email, but I see this
>> in patchwork.
>>
>> For me this cut build time from 6.25 minutes to 4.75 minutes for 30
>> commits built for two similar boards on a 8-thread machine. It's a big
>> win. I haven't yet tried it out on a a big build.
>>
>> Stephen can you please update the README with some of your commit
>> notes? Other than that:
>>
>> Acked-by: Simon Glass <sjg at chromium.org>
>> Tested-by: Simon Glass <sjg at chromium.org>
>
>

Regards,
Simon