arduino / arduino-cli

Arduino command line tool
https://arduino.github.io/arduino-cli/latest/
GNU General Public License v3.0
4.33k stars 374 forks source link

Improved compile speed by running multi-threaded library discovery. #2625

Open cmaglie opened 3 months ago

cmaglie commented 3 months ago

Please check if the PR fulfills these requirements

See how to contribute

What kind of change does this PR introduce?

This is a tentative to improve compile speed by multi-threading the library discovery phase.

How library discovery works

The library discovery is a sequential process, and could be summarized as follows:

The above loop continues until one of the following happens:

Could this be multi-threaded?

The main issue here is that each run of gcc -E ... requires the includes path list determined by the previous iterations. At first sight, it seems a strictly sequential process.

BTW, there are some strategies that we may use:

  1. When a library is added to the queue, we may speculatively start a multithreaded compilation of the next files in the queue assuming no missing include errors will be produced. Most of the time this assumption turns out to be true and the time saved could be dramatic.

  2. We may leverage the "library detection cache" to spawn a speculative multithreaded compilation of all the files (predicting all the libraries added in the process), assuming that the missing include errors would be the same as the previous successful compilation as saved in the "library detection cache". In this case, we reproduce the same compiles as the previous build. The basic assumption is that the libraries detected will not change between compilations.

If an unforeseen missing include error is detected, the speculative multithreaded compilation must be canceled and restarted (basically wasting the work done ahead of time). In a multi-core system, since the work "ahead" is done in parallel, the worst-case scenario should take nearly the same time as the "classic" non-multithreaded compile.

Due to the complexity of these changes, I'll start this as a draft. I want to add integration tests to trigger edge cases before merging.

What is the current behavior?

What is the new behavior?

Compiles should in general be faster than before.

Does this PR introduce a breaking change, and is titled accordingly?

No

Other information

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 77.82910% with 96 lines in your changes missing coverage. Please review.

Project coverage is 67.87%. Comparing base (ce4341f) to head (87765a4).

Files Patch % Lines
...lder/internal/preprocessor/arduino_preprocessor.go 0.00% 22 Missing :warning:
...nternal/arduino/builder/internal/detector/cache.go 72.46% 14 Missing and 5 partials :warning:
...rnal/arduino/builder/internal/detector/detector.go 85.04% 9 Missing and 7 partials :warning:
internal/arduino/builder/sizer.go 12.50% 6 Missing and 1 partial :warning:
internal/arduino/builder/builder.go 82.35% 4 Missing and 2 partials :warning:
...nal/arduino/builder/internal/preprocessor/ctags.go 62.50% 6 Missing :warning:
internal/arduino/builder/compilation.go 86.84% 3 Missing and 2 partials :warning:
...l/arduino/builder/internal/detector/source_file.go 88.63% 3 Missing and 2 partials :warning:
commands/service_compile.go 72.72% 2 Missing and 1 partial :warning:
internal/arduino/builder/internal/runner/task.go 80.00% 2 Missing and 1 partial :warning:
... and 3 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2625 +/- ## ========================================== + Coverage 67.67% 67.87% +0.20% ========================================== Files 234 237 +3 Lines 22176 22269 +93 ========================================== + Hits 15007 15116 +109 + Misses 5990 5978 -12 + Partials 1179 1175 -4 ``` | [Flag](https://app.codecov.io/gh/arduino/arduino-cli/pull/2625/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arduino) | Coverage Δ | | |---|---|---| | [unit](https://app.codecov.io/gh/arduino/arduino-cli/pull/2625/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arduino) | `67.87% <77.82%> (+0.20%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arduino#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

manchoz commented 3 months ago

LGTM