[tools] Parallel building of tests #2990

bridadan · 2016-10-11T20:44:29Z

Description

tl;dr: the linking step of building tests now respects the -j flag, so it can be parallelized to make building tests faster.

This uses similar code that is used withing the toolchains when compiling to parallelize the linking process of all the tests across the available CPUs.

To prevent interleaving of build output, I had to modify how the toolchains handle the silent parameter. @screamerbg or @theotherjimmy if this will cause issues, please let me know and I will add a different parameter to stop the printing from occurring.

Statistics

Some brief numbers, cuz who doesn't like numbers :)

Before parallelization, building all tests for the K64F with GCC_ARM on our "Build beast" CI machine took ~45 seconds, with almost 40 seconds of that taken up by the linking stage.

After parallelization, the same build now takes ~6 seconds!

Status

READY

Migrations

If this PR changes any APIs or behaviors, give a short description of what API users should do when this PR is merged.

NO

Todos

Tests
Review by @screamerbg
Review by @theotherjimmy
Review by @MarceloSalazar (memap changes)

Deploy notes

This will consume more resources on a host machine for a shorter amount of time. But this should be taken into consideration in CI.

This uses similar code that is used withing the toolchains to parallelize the linking process of all the tests accross all the available CPUs. It also respects the `-j` parameter if you wish to limit the number of cores used.

The 'silent' option has always been present in the toolchains API, however it did not actually stop anything from being printed. Instead, it just changed what was added to the build log. This make the 'silent' stop all prints, but ensures that the output for the toolchain is still preserved and accessible via the 'get_output' function.

This makes use of the reports generated by the building of tests to prevent output from interleaving when the build is parallelized. This required some changes to memap to return a generated string from the 'generate_output' function. I also had an option to stop the prints from memap to prevent text from interleaving

bridadan · 2016-10-11T20:44:55Z

/morph test

mbed-bot · 2016-10-11T20:45:53Z

Result: ABORTED

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 0

Build Prep failed!

bridadan · 2016-10-11T20:47:12Z

Attention maintainers: Please refrain from triggering testing on this for the time being. There are some crucial tests running at the moment and I'd prefer not to hurt them by burning up the CPUs with this bad boy 😄 I'll start the testing when the machines are free.

screamerbg · 2016-10-11T21:11:08Z

/morphhhhh...

@bridadan okay :)

screamerbg

Please address the in-line comments

screamerbg · 2016-10-11T20:58:17Z

tools/toolchains/__init__.py

@@ -1075,7 +1076,7 @@ def mem_stats(self, map):
            return None

        # Write output to stdout in text (pretty table) format
-        memap.generate_output('table')
+        self.info(memap.generate_output('table', silent=True))


Memap generated data is not supposed to be passed via self.info(). This is a backwards compatibility breaking change and won't work with the online build system. Rather than doing this I'd suggest that this is either:

[thread unsafe] added to the toolchain e.g. toolchain.memap_output = memap.generate_output('table'); or

[thread safe] passed as part of the return result

Good to know, glad I asked ahead of time. I'll explore the thread safe option.

screamerbg · 2016-10-11T21:02:34Z

tools/memap.py

@@ -9,6 +9,7 @@
 import json
 import argparse
 from prettytable import PrettyTable
+from StringIO import StringIO


Think we should rather reuse what we have and not add new unique python class that will be included to every build, export, etc? Let's use what we have here @bridadan and minimize dependencies :)

This was used to allow the csv exporter to write to a string (so it can be returned). The way it is currently written there is no way to get the output as a string before writing it to a file. This may not actually be a big deal though, so I will revisit this.

screamerbg · 2016-10-11T21:06:51Z

tools/memap.py

+                if file_output:
+                    file_desc = open(file_output, 'wb')
+                else:
+                    file_desc = sys.stdout


A bit too much much if->if->then->else. I understand that the new code requires that memap generates string output, not write to files, therefore I'd expect that the memap is split into:

methods that generate string output

methods that actually write or print output

The proposed implementation is a spaghetti mix of these

Fair point, I'll go over it again. Might be able to simplify it a bit.

screamerbg · 2016-10-11T21:08:15Z

tools/test_api.py

@@ -2068,6 +2069,40 @@ def norm_relative_path(path, start):
    path = path.replace("\\", "/")
    return path

+
+def build_test_worker(*args, **kwargs):


Sweet! 👍

screamerbg · 2016-10-11T21:08:34Z

tools/test_api.py

+            else:
+                pending += 1
+                if pending >= jobs_count:
+                    break


bridadan · 2016-10-11T23:47:26Z

tools/toolchains/__init__.py


        if msg:
-            print msg
+            if not silent:
+                print msg


@screamerbg Do you anticipate this change causing any problems?

Should be fine. CC @theotherjimmy

Should be fine.

bridadan · 2016-10-11T23:47:50Z

tools/toolchains/__init__.py

@@ -396,7 +396,8 @@ def print_notify_verbose(self, event, silent=False):
            event['target_name'] = event['target_name'].upper() if event['target_name'] else "Unknown"
            event['toolchain_name'] = event['toolchain_name'].upper() if event['toolchain_name'] else "Unknown"
            msg = '[%(severity)s] %(target_name)s::%(toolchain_name)s::%(file)s@%(line)s: %(message)s' % event
-            print msg
+            if not silent:
+                print msg


@screamerbg Do you anticipate this change causing any problems?

bridadan · 2016-10-11T23:47:52Z

tools/toolchains/__init__.py

            self.output += msg + "\n"

    def print_notify_verbose(self, event, silent=False):
        """ Default command line notification with more verbose mode
        """
        if event['type'] in ['info', 'debug']:
-            self.print_notify(event) # standard handle
+            self.print_notify(event, silent=silent) # standard handle


@screamerbg Do you anticipate this change causing any problems?

Nope. Looks fine.

bridadan · 2016-10-11T23:50:10Z

Ok, I've since addressed your concerns I believe @screamerbg, would you mind reviewing again?

/morph test

mbed-bot · 2016-10-12T01:05:34Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 0

All builds and test passed!

bridadan · 2016-10-12T14:34:11Z

From CI, the build process went from 17 minutes to 13 minutes. And I bet I can get the down farther after optimize which jobs get ran where. I would expect to get it closer to 10 minutes.

screamerbg · 2016-10-14T11:04:38Z

@bridadan Really awesome work! Sorry for delayed review.

LGTM
@sg-

screamerbg · 2016-10-14T11:03:16Z

tools/toolchains/__init__.py


        if msg:
-            print msg
+            if not silent:
+                print msg


Should be fine. CC @theotherjimmy

screamerbg · 2016-10-14T11:03:36Z

tools/toolchains/__init__.py

            self.output += msg + "\n"

    def print_notify_verbose(self, event, silent=False):
        """ Default command line notification with more verbose mode
        """
        if event['type'] in ['info', 'debug']:
-            self.print_notify(event) # standard handle
+            self.print_notify(event, silent=silent) # standard handle


Nope. Looks fine.

screamerbg · 2016-10-14T11:03:53Z

tools/toolchains/__init__.py

-        map_csv = splitext(map)[0] + "_map.csv"
-        memap.generate_output('csv-ci', map_csv)
+        # Store the memap instance for later use
+        self.memap_instance = memap


jeromecoutant · 2016-10-19T11:13:15Z

Hi
How can we get this improvement ?
Is there any parameter to add in the build command in our side?
Thx

bridadan · 2016-10-19T14:00:23Z

@jeromecoutant The linking step will now obey the -j flag, which by default is set to "auto detect how many cpus are available and use all of them". So you shouldn't have to change anything. If you wish to limit how many cpus are used, then you can still use the -j flag to do this.

Release mbed OS 5.2.2 and mbed lib v129 Known issues in this release There is currently a DNS resolution failure in Thread mode with this release. This causes a failure in the mbed-os-example-client. This will be fixed in a subsequent release. This can be worked around by reverting to mbed-os-5.2.0 Ports for Upcoming Targets 3011: Add u-blox Sara-N target. ARMmbed#3011 3099: MAX32625 ARMmbed#3099 3151: Add support for FRDM-K82F ARMmbed#3151 3177: New mcu k22512 fixing pr 3136 ARMmbed#3177 Fixes and Changes 2990: [tools] Parallel building of tests ARMmbed#2990 3008: NUCLEO_F072RB: Fix wrong timer channel number on pwm PB_5 pin ARMmbed#3008 3013: STM32xx - Change how the ADC internal pins are checked before pinmap_ ARMmbed#3013 3023: digital_loop tests update for STM32 ARMmbed#3023 3041: [nRF5] - added implementation of API of serial port flow control configuration. ARMmbed#3041 3092: [tools + tests] Adding parallelized build option for iar and uvision exporters ARMmbed#3092 3084: [nrf5] fix in Digital I/O : a gpioe pin was uninitialized badly ARMmbed#3084 3009: TRNG enabled. TRNG APIs implemented. REV A/B/C/D flags removed. Warnings removed ARMmbed#3009 3139: Handle [NOT_SUPPORTED] exception in make.py ARMmbed#3139 3074: Target stm init gcc alignement ARMmbed#3074 3140: [tests] Replacing getchar with RawSerial getc in greentea-client ARMmbed#3140 3158: Added support for 6lowpan PAN ID filter to mbed mesh api configuration ARMmbed#3158 2988: Update of can_api.c fixing ARMmbed#2987 ARMmbed#2988 3175: Updating IAR definition for the NCS36510 for IAR EW v7.8 ARMmbed#3175 3170: [tests] Preventing test from printing before Greentea __sync ARMmbed#3170 3169: [Update of ARMmbed#3014] Usb updates ARMmbed#3169 3143: CFStore fix needed for the Cloud Client ARMmbed#3143 3135: lwip - Fix memory leak in k64f cyclic-buffer overflow ARMmbed#3135 3048: Make update.py test compile examples prior to updating mbed-os version. ARMmbed#3048 3162: lwip/nsapi - Clean up warnings in network code ARMmbed#3162 3161: nsapi - Add better heuristic for the default record of DNS queries ARMmbed#3161 3173: [Exporters] Add a device_name to microbit entry in targets.json ARMmbed#3173 3072: i2c_loop tests update for STM32 ARMmbed#3072 2958: Allowing mbed_app.json files to be discovered for tests. ARMmbed#2958 2969: [nRF52] - switch irq priorities of driver handlers to the lowest level ARMmbed#2969 3078: lwip: Allow several configuration macros to be set externally (bis) ARMmbed#3078 3165: Add address type checks to NanostackInterface ARMmbed#3165 3166: nsapi_dns: Provide 2 IPv6-hosted default servers ARMmbed#3166 3171: [tools] Fixing project.py -S printing problem ARMmbed#3171 3172: [Exporters] New export-build tests ARMmbed#3172 3184: ARMmbed#3183 Compiler warning in trng_api.c with K64F ARMmbed#3184 3185: Update tests to fix build failures. Also make the code similar to oth ARMmbed#3185 3104: [NuMaker] Support CAN and fix PWM CLK error ARMmbed#3104 3182: Exporter documentation ARMmbed#3182 3186: MultiTech mDot - add back SPI3 pins ARMmbed#3186 3187: [Export-Make] Use internal class variable for resolving templates in makefiles ARMmbed#3187 3195: [Exporters - Make-based] Quote the shell call in mkdir and rmdir ARMmbed#3195 3204: [Export build-test] Directory traversal error ARMmbed#3204 3189: [Exporters - Make-based] Force make exporter to search PATH for compilers ARMmbed#3189 3200: Using Popen for uVision and unifying the structure of the build function ARMmbed#3200 3075: nsapi - Add standardized return types for size and errors ARMmbed#3075 3221: u-blox odin w2 drivers update ARMmbed#3221

bridadan added 3 commits October 11, 2016 13:24

Making building and linking tests fully parallel.

87103ab

This uses similar code that is used withing the toolchains to parallelize the linking process of all the tests accross all the available CPUs. It also respects the `-j` parameter if you wish to limit the number of cores used.

bridadan added the needs: review label Oct 11, 2016

screamerbg suggested changes Oct 11, 2016

View reviewed changes

bridadan added 2 commits October 11, 2016 18:24

Removing need for memap to printed through toolchains

0e4b841

Removing dependency on StringIO, only change return type of table export

3908672

bridadan commented Oct 11, 2016

View reviewed changes

screamerbg added ready for merge and removed needs: review labels Oct 14, 2016

screamerbg approved these changes Oct 14, 2016

View reviewed changes

theotherjimmy approved these changes Oct 17, 2016

View reviewed changes

sg- added the release-version: 5.2.2 label Oct 19, 2016

sg- merged commit 6bd44c5 into ARMmbed:master Oct 19, 2016

sg- removed the ready for merge label Oct 19, 2016

bridadan mentioned this pull request Oct 19, 2016

mbed test : is it possible to use parallel compilation ? ARMmbed/mbed-cli#343

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tools] Parallel building of tests #2990

[tools] Parallel building of tests #2990

bridadan commented Oct 11, 2016 •

edited by theotherjimmy

Loading

bridadan commented Oct 11, 2016

mbed-bot commented Oct 11, 2016

bridadan commented Oct 11, 2016

screamerbg commented Oct 11, 2016

screamerbg left a comment

screamerbg Oct 11, 2016 •

edited

Loading

bridadan Oct 11, 2016

screamerbg Oct 11, 2016

bridadan Oct 11, 2016 •

edited

Loading

screamerbg Oct 11, 2016 •

edited

Loading

bridadan Oct 11, 2016

screamerbg Oct 11, 2016

screamerbg Oct 11, 2016

bridadan Oct 11, 2016 •

edited

Loading

screamerbg Oct 14, 2016

theotherjimmy Oct 17, 2016

bridadan Oct 11, 2016

bridadan Oct 11, 2016

screamerbg Oct 14, 2016

bridadan commented Oct 11, 2016

mbed-bot commented Oct 12, 2016

bridadan commented Oct 12, 2016

screamerbg commented Oct 14, 2016

screamerbg Oct 14, 2016

screamerbg Oct 14, 2016

screamerbg Oct 14, 2016

jeromecoutant commented Oct 19, 2016

bridadan commented Oct 19, 2016

[tools] Parallel building of tests #2990

[tools] Parallel building of tests #2990

Conversation

bridadan commented Oct 11, 2016 • edited by theotherjimmy Loading

Description

Statistics

Status

Migrations

Todos

Deploy notes

bridadan commented Oct 11, 2016

mbed-bot commented Oct 11, 2016

Output

bridadan commented Oct 11, 2016

screamerbg commented Oct 11, 2016

screamerbg left a comment

Choose a reason for hiding this comment

screamerbg Oct 11, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bridadan Oct 11, 2016 • edited Loading

Choose a reason for hiding this comment

screamerbg Oct 11, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bridadan Oct 11, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bridadan commented Oct 11, 2016

mbed-bot commented Oct 12, 2016

Output

bridadan commented Oct 12, 2016

screamerbg commented Oct 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeromecoutant commented Oct 19, 2016

bridadan commented Oct 19, 2016

bridadan commented Oct 11, 2016 •

edited by theotherjimmy

Loading

screamerbg Oct 11, 2016 •

edited

Loading

bridadan Oct 11, 2016 •

edited

Loading

screamerbg Oct 11, 2016 •

edited

Loading

bridadan Oct 11, 2016 •

edited

Loading