Skip to content

Support for multiple compile queues/jobs, stats cache and compile order consistency #395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Aug 7, 2014
Merged

Conversation

screamerbg
Copy link
Contributor

This patch set adds:

  • compile order consistency across filesystem changes
  • stats cache for need_update() routine to speed things up and reduce IO load
  • ability to compile libraries and programs over multiple host cores/CPUs

The compile queues feature is controlled via -j flag to build.py, build_releases.py and make.py. The default value is 1 which effectively disables the support. If -j 0 is specified, then the number of host machines CPUs will be used to determine the number of jobs.

@bogdanm
Copy link
Contributor

bogdanm commented Jul 10, 2014

This can't be merged yet, it has a few important problems:

  • "-v" (verbose) flag of make.py and build.py doesn't work properly anymore, not even when "-j" is not used. This is important, as the first thing to do after a failed compilation is often to check the compiler command line.
  • I can't compile using IAR anymore (build.py -t IAR -m LPC1768). That happens most likely because there isn't a "self.assemble" call anymore in toolchains/__init__py, which did the right thing for IAR (I suspect this might break things in other toolchains too).
  • the calls to self.assemble, self.compile_c, self.compile_cpp in toolchain/init.py were meant to be implemented in an OOP inheritance fashion, in which each toolchain implementation could change them as needed. Now this doesn't happen anymore, everything is handled in a single method in the base class (_compile_command) which assumes that there's a single command line that can be used to compile something, no matter what the actual toolchain is. While this might be true at the moment, it defeats the purpose of the initial inheritance-based design: let each toolchain design how it needs to compile/assemble its sources.

@screamerbg
Copy link
Contributor Author

  • The "-v" flag is a bug and will be fixed
  • The self.assemble is not implemented in toolchains/init.py, take a look here https://github.com/mbedmicro/mbed/blob/master/workspace_tools/toolchains/__init__.py. I'll take a look at the problem with IAR and fix it.
  • The concept of separation between .s, .c, .cpp file is unnecessary over-complication of the toolchain compile logic. And as you say there isn't and there won't be a C/CPP toolchain that deviates from what is established currently and require more than one command to compile a single source file. :)

…ancing based on apply_async();

Use the returned result by apply_async() to fetch compile_worker() results and get rid of python queues;
Optimize the threads handling code
Reuse compile threads via self.mp_pool
Added get_dep_opt for ARM class
@screamerbg
Copy link
Contributor Author

  • "-v" is now fixed
  • compile with IAR is also fixed
  • the multiprocessing code uses a more lightweight apply_async() method that also load balances tasks between threads

@bogdanm
Copy link
Contributor

bogdanm commented Aug 5, 2014

I've been looking at merging this lately, but I've found a new problem and I'm not sure what's the best way to fix this. The problem is that the "assemble" method for ARM is actually a two step process: it first preprocesses the file, and then assembles the output of the preprocessor:

    def assemble(self, source, object, includes):
        # Preprocess first, then assemble
        tempfile = object + '.E.s'
        self.default_cmd(self.asm + ['-D%s' % s for s in self.get_symbols() + self.macros] + ["-I%s" % i for i in includes] + ["-E", "-o", tempfile, source])
        self.default_cmd(self.hook.get_cmdline_assembler(self.asm + ["-o", object, tempfile]))

I added this in commit e69956a in order to be able to compile the new (back then) CMSIS-DSP library. If you try to compile the DSP library with your patch in place (build.py -t ARM -m LPC1768 -d), you'll see that it fails because this change is now ignored. Integrating it is problematic, because as it is above the "assemble" step of ARM actually consists of two separate commands (preprocess and then assemble) which must run in order (assemble needs to run after preprocess finishes). Can you think of a simple way to integrate this into your architecture? By the way, this illustrates pretty well why "The concept of separation between .s, .c, .cpp file is unnecessary over-complication of the toolchain compile logic" is false. There's no good point in making asumptions about how a particular toolchain works, it's much better to let the toolchain implementation worry about its internal details.

…ds per source file

Support multiple commands per compile
Reuse _assemble() and _compile() for sequential and parallel compiles
Preserve compile(), compile_c(), compile_cpp() and assemble() methods functionality
if itr > 6000:
p.terminate()
p.join()
raise ToolException("Compile did not finish in 5 minutes")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand '5 minutes' here. You are executing this loop with a 10 ms delay at the end of the loop for 6000 times, which translates to 1 minute.

bogdanm added a commit that referenced this pull request Aug 7, 2014
Support for multiple compile queues/jobs, stats cache and compile order consistency
@bogdanm bogdanm merged commit 2df4afd into ARMmbed:master Aug 7, 2014
yogpan01 pushed a commit to yogpan01/mbed that referenced this pull request Jul 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants