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

* [U-Boot] [PATCH V2] buildman: allow more incremental building
@ 2016-04-11 16:48 Stephen Warren
  2016-04-25 16:41 ` Stephen Warren
  2016-05-04 18:55 ` Stephen Warren
  0 siblings, 2 replies; 10+ messages in thread
From: Stephen Warren @ 2016-04-11 16:48 UTC (permalink / raw)
  To: u-boot

From: Stephen Warren <swarren@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.

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 <swarren@nvidia.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
Acked-by: Simon Glass <sjg@chromium.org> # v1
Tested-by: Simon Glass <sjg@chromium.org> # v1
---
v2: Add README updates.
---
 tools/buildman/README           | 42 +++++++++++++++++++++++++++++++++++++++++
 tools/buildman/builder.py       | 10 ++++++++--
 tools/buildman/builderthread.py | 24 +++++++++++++++--------
 tools/buildman/cmdline.py       |  4 ++++
 tools/buildman/control.py       |  4 +++-
 5 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/tools/buildman/README b/tools/buildman/README
index 4705d2644b8f..26755c5955e2 100644
--- a/tools/buildman/README
+++ b/tools/buildman/README
@@ -898,6 +898,48 @@ when using the -b flag. For example:
 will build commits in us-buildman that are not in upstream/master.
 
 
+Building Faster
+===============
+
+By default, buildman executes 'make mrproper' prior to building the first
+commit for each board. This causes everything to be built from scratch. If you
+trust the build system's incremental build capabilities, you can pass the -I
+flag to skip the 'make mproper' invocation, which will reduce the amount of
+work 'make' does, and hence speed up the build. This flag will speed up any
+buildman invocation, since it reduces the amount of work done on any build.
+
+One possible application of buildman is as part of a continual edit, build,
+edit, build, ... cycle; repeatedly applying buildman to the same change or
+series of changes while making small incremental modifications to the source
+each time. This provides quick feedback regarding the correctness of recent
+modifications. In this scenario, buildman's default choice of build directory
+causes more build work to be performed than strictly necessary.
+
+By default, each buildman thread uses a single directory for all builds. When a
+thread builds multiple boards, the configuration built in this directory will
+cycle through various different configurations, one per board built by the
+thread. Variations in the configuration will force a rebuild of affected source
+files when a thread switches between boards. Ideally, such buildman-induced
+rebuilds would not happen, thus allowing the build to operate as efficiently as
+the build system and source changes allow. buildman's -P flag may be used to
+enable this; -P causes each board to be built in a separate (board-specific)
+directory, thus avoiding any buildman-induced configuration changes in any
+build directory.
+
+U-Boot's build system embeds information such as a build timestamp into the
+final binary. This information varies each time U-Boot is built. This causes
+various files to be rebuilt even if no source changes are made, which in turn
+requires that the final U-Boot binary be re-linked. This unnecessary work can
+be avoided by turning off the timestamp feature. This can be achieved by
+setting the SOURCE_DATE_EPOCH environment variable to 0.
+
+Combining all of these options together yields the command-line shown below.
+This will provide the quickest possible feedback regarding the current content
+of the source tree, thus allowing rapid tested evolution of the code.
+
+    SOURCE_DATE_EPOCH=0 ./tools/buildman/buildman -I -P tegra
+
+
 Other options
 =============
 
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 141bf6469136..8ec355172901 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -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)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index cf25bb8f1a7e..c512d3b521f7 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -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
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 8341ab145cfa..3e3bd63e32c7 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -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',
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index c2c54bf0e81b..aeb128a6a3e9 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -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
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread