buildman: allow more incremental building

PATCH New Stephen Warren 3 participants 6 messages Mar 28, 2016 – Apr 11, 2016

lore

Share & tools

lore

Stephen Warren

author

2016-03-28 22:39 UTC #0 lore raw

PATCH 1/1 buildman: allow more incremental building

From: Stephen Warren <[email protected]>

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.

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.

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 <[email protected]>

---
 tools/buildman/builder.py       | 10 ++++++++--
 tools/buildman/builderthread.py | 24 ++++++++++++++++--------
 tools/buildman/cmdline.py       |  4 ++++
 tools/buildman/control.py       |  4 +++-
 4 files changed, 31 insertions(+), 11 deletions(-)

tools/buildman/builder.py +8 -2

index 141bf6469136..8ec355172901 100644
@@ -205,7 +205,8 @@ class Builder:
 
     def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs,
                  gnu_make='make', checkout=True, show_unknown=True, step=1,
-                 no_subdirs=False, full_path=False, verbose_build=False):
+                 no_subdirs=False, full_path=False, verbose_build=False,
+                 incremental=False, per_board_out_dir=False):
         """Create a new Builder object
 
         Args:

@@ -224,6 +225,10 @@ class Builder:
             full_path: Return the full path in CROSS_COMPILE and don't set
                 PATH
             verbose_build: Run build with V=1 and don't use 'make -s'
+            incremental: Always perform incremental builds; don't run make
+                mrproper when configuring
+            per_board_out_dir: Build in a separate persistent directory per
+                board rather than a thread-specific directory
         """
         self.toolchains = toolchains
         self.base_dir = base_dir

@@ -263,7 +268,8 @@ class Builder:
         self.queue = Queue.Queue()
         self.out_queue = Queue.Queue()
         for i in range(self.num_threads):
-            t = builderthread.BuilderThread(self, i)
+            t = builderthread.BuilderThread(self, i, incremental,
+                    per_board_out_dir)
             t.setDaemon(True)
             t.start()
             self.threads.append(t)

tools/buildman/builderthread.py +16 -8

index cf25bb8f1a7e..c512d3b521f7 100644
@@ -80,11 +80,13 @@ class BuilderThread(threading.Thread):
         thread_num: Our thread number (0-n-1), used to decide on a
                 temporary directory
     """
-    def __init__(self, builder, thread_num):
+    def __init__(self, builder, thread_num, incremental, per_board_out_dir):
         """Set up a new builder thread"""
         threading.Thread.__init__(self)
         self.builder = builder
         self.thread_num = thread_num
+        self.incremental = incremental
+        self.per_board_out_dir = per_board_out_dir
 
     def Make(self, commit, brd, stage, cwd, *args, **kwargs):
         """Run 'make' on a particular commit and board.

@@ -136,7 +138,11 @@ class BuilderThread(threading.Thread):
         if self.builder.in_tree:
             out_dir = work_dir
         else:
-            out_dir = os.path.join(work_dir, 'build')
+            if self.per_board_out_dir:
+                out_rel_dir = os.path.join('..', brd.target)
+            else:
+                out_rel_dir = 'build'
+            out_dir = os.path.join(work_dir, out_rel_dir)
 
         # Check if the job was already completed last time
         done_file = self.builder.GetDoneFile(commit_upto, brd.target)

@@ -197,12 +203,12 @@ class BuilderThread(threading.Thread):
                         #
                         # Symlinks can confuse U-Boot's Makefile since
                         # we may use '..' in our path, so remove them.
-                        work_dir = os.path.realpath(work_dir)
-                        args.append('O=%s/build' % work_dir)
+                        out_dir = os.path.realpath(out_dir)
+                        args.append('O=%s' % out_dir)
                         cwd = None
                         src_dir = os.getcwd()
                     else:
-                        args.append('O=build')
+                        args.append('O=%s' % out_rel_dir)
                 if self.builder.verbose_build:
                     args.append('V=1')
                 else:

@@ -215,9 +221,11 @@ class BuilderThread(threading.Thread):
 
                 # If we need to reconfigure, do that now
                 if do_config:
-                    result = self.Make(commit, brd, 'mrproper', cwd,
-                            'mrproper', *args, env=env)
-                    config_out = result.combined
+                    config_out = ''
+                    if not self.incremental:
+                        result = self.Make(commit, brd, 'mrproper', cwd,
+                                'mrproper', *args, env=env)
+                        config_out += result.combined
                     result = self.Make(commit, brd, 'config', cwd,
                             *(args + config_args), env=env)
                     config_out += result.combined

tools/buildman/cmdline.py +4 -0

index 8341ab145cfa..3e3bd63e32c7 100644
@@ -49,6 +49,8 @@ def ParseArgs():
     parser.add_option('-i', '--in-tree', dest='in_tree',
           action='store_true', default=False,
           help='Build in the source tree instead of a separate directory')
+    parser.add_option('-I', '--incremental', action='store_true',
+          default=False, help='Do not run make mrproper (when reconfiguring)')
     parser.add_option('-j', '--jobs', dest='jobs', type='int',
           default=None, help='Number of jobs to run at once (passed to make)')
     parser.add_option('-k', '--keep-outputs', action='store_true',

@@ -70,6 +72,8 @@ def ParseArgs():
           default=False, help='Do a rough build, with limited warning resolution')
     parser.add_option('-p', '--full-path', action='store_true',
           default=False, help="Use full toolchain path in CROSS_COMPILE")
+    parser.add_option('-P', '--per-board-out-dir', action='store_true',
+          default=False, help="Use an O= (output) directory per board rather than per thread")
     parser.add_option('-s', '--summary', action='store_true',
           default=False, help='Show a build summary')
     parser.add_option('-S', '--show-sizes', action='store_true',

tools/buildman/control.py +3 -1

index c2c54bf0e81b..aeb128a6a3e9 100644
@@ -250,7 +250,9 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
             options.threads, options.jobs, gnu_make=gnu_make, checkout=True,
             show_unknown=options.show_unknown, step=options.step,
             no_subdirs=options.no_subdirs, full_path=options.full_path,
-            verbose_build=options.verbose_build)
+            verbose_build=options.verbose_build,
+            incremental=options.incremental,
+            per_board_out_dir=options.per_board_out_dir,)
     builder.force_config_on_failure = not options.quick
     if make_func:
         builder.do_make = make_func

Signature · 1 line

2.7.4

Tom Rini

2016-04-01 13:35 UTC #1 lore raw

PATCH 1/1 buildman: allow more incremental building

Quoted 31 lines · Stephen Warren

From: Stephen Warren <[email protected]>
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.
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.
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

Quoted 4 lines

... each once after deleting the buildman result/work directory, and once "incrementally" after a previous identical invocation.
Signed-off-by Stephen Warren <[email protected]>

Reviewed-by Tom Rini <[email protected]>

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/0d17a5e4/attachment.sig>

Simon Glass

2016-04-11 12:04 UTC #2 lore raw

PATCH 1/1 buildman: allow more incremental building

Hi,

Quoted 10 lines · Tom Rini

Quoted 8 lines · Stephen Warren
From: Stephen Warren <[email protected]>
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:

Quoted 4 lines

Quoted 4 lines
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.

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.

Quoted 6 lines

Quoted 6 lines
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.

Quoted 13 lines

Quoted 13 lines
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

Quoted 6 lines

Quoted 4 lines
... each once after deleting the buildman result/work directory, and once "incrementally" after a previous identical invocation.
Signed-off-by Stephen Warren <[email protected]>
Reviewed-by Tom Rini <[email protected]>

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 <[email protected]>

Tested-by Simon Glass <[email protected]>

Regards,
Simon

Stephen Warren

author

2016-04-11 16:15 UTC #3 lore raw

PATCH 1/1 buildman: allow more incremental building

Quoted 13 lines · Simon Glass

Hi,
Quoted 10 lines · Tom Rini
Quoted 8 lines · Stephen Warren
From: Stephen Warren <[email protected]>
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:

Quoted 7 lines

Quoted 4 lines
Quoted 4 lines
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.

Quoted 4 lines

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.

Quoted 8 lines

Quoted 5 lines
Quoted 5 lines
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.

Quoted 31 lines

Quoted 12 lines
Quoted 12 lines
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
Quoted 6 lines
Quoted 4 lines
... each once after deleting the buildman result/work directory, and once "incrementally" after a previous identical invocation.
Signed-off-by Stephen Warren <[email protected]>
Reviewed-by Tom Rini <[email protected]>
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 <[email protected]>
Tested-by Simon Glass <[email protected]>

Simon Glass

2016-04-11 16:58 UTC #4 lore raw

PATCH 1/1 buildman: allow more incremental building

Hi Stephen,

Quoted 29 lines · Stephen Warren

Quoted 24 lines · Simon Glass
Hi,
Quoted 13 lines · Tom Rini
Quoted 10 lines · Stephen Warren
From: Stephen Warren <[email protected]>
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:
Quoted 5 lines
Quoted 5 lines
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.
Quoted 2 lines
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.

Quoted 5 lines

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.

Quoted 43 lines

Quoted 5 lines
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.
Quoted 10 lines
Quoted 8 lines
Quoted 8 lines
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.
Quoted 2 lines
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

Quoted 3 lines

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?

Quoted 32 lines

Quoted 32 lines
Quoted 15 lines
Quoted 15 lines
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
Quoted 8 lines
Quoted 5 lines
... each once after deleting the buildman result/work directory, and once
"incrementally" after a previous identical invocation.
Signed-off-by Stephen Warren <[email protected]>
Reviewed-by Tom Rini <[email protected]>
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.

Quoted 7 lines

Quoted 5 lines
Stephen can you please update the README with some of your commit notes? Other than that:
Acked-by Simon Glass <[email protected]>
Tested-by Simon Glass <[email protected]>

Regards,
Simon

Stephen Warren

author

2016-04-11 17:07 UTC #5 lore raw

PATCH 1/1 buildman: allow more incremental building

Quoted 82 lines · Simon Glass

Hi Stephen,
Quoted 29 lines · Stephen Warren
Quoted 24 lines · Simon Glass
Hi,
Quoted 13 lines · Tom Rini
Quoted 10 lines · Stephen Warren
From: Stephen Warren <[email protected]>
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:
Quoted 5 lines
Quoted 5 lines
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.
Quoted 2 lines
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.
Quoted 5 lines
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.
Quoted 43 lines
Quoted 5 lines
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.
Quoted 10 lines
Quoted 8 lines
Quoted 8 lines
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.
Quoted 2 lines
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

Quoted 7 lines

Quoted 3 lines
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?

Each thread can build multiple boards in turn. This is not a problem building a series of commits for a board, but rather a problem when building multiple boards in turn. Also, see V2 of the patch which adds a longer explanation to the README.