Skip to content
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

Switch to EPP analysis covering all apps at once when compiling #2236

Merged
merged 8 commits into from
Mar 8, 2020

Conversation

ferd
Copy link
Collaborator

@ferd ferd commented Feb 20, 2020

This continues the Compiler refactor sequence in #2200 and switches code analysis to be done with EPP, which can understand macros and conditionally expand paths.

At the same time, this PR expands things to analyze all project Apps as one unit, which in turn allows properly detecting cases such as behaviours, parse transform, or include files changing in other applications forcing a re-build of the local ones.

The one limitation is that EPP won't easily allow the case where a public include file includes private include files, and the public include file is included by a foreign module; in such a case changes to the private headers won't be seen outside of their original app.

The apps are sorted in proper build order according to the new DAG, and should allow for interesting parallelism options down the line as well.

Preliminary tests on larger code bases seem to imply that this slightly slows down the initial build but significantly speeds up the following partial builds.

ferd added 4 commits February 20, 2020 16:44
This commit is a transition point that makes some assumptions about new
callbacks for the compiler modules, which will likely not hold when it
comes to making epp be able to read and handle multiple applications at
once (so it can resolve parse transforms and behaviours across OTP
apps).

As such, a follow-up commit is going to be completing that one and
changing its API in incompatible ways; if you find this commit during a
bisect, it's probably not a good one in which to run a bisection.
While leaving the old one in place, prep the ground for new analysis
phases for the DAG work. The new DAG functions are added, but not hooked
in yet.

Fixes to the EPP handling have also been added due to issues in
resolving include_lib information
This moves the old DAG stuff out from the main code paths and
substitutes it with the new epp-based DAG work.

Also fixes previous work for integration tests:

- consider ptrans from erl_opts in app ordering
- display proper app compilation info messages due to reordering
@ferd
Copy link
Collaborator Author

ferd commented Feb 20, 2020

Oh another gotcha: I didn't get rid of all the old ass code to parse attributes in rebar_compiler_erl because right now the dependencies/3 callback is mandatory and that call is needed for that version to work, but the old code is not used at all.

@ferd
Copy link
Collaborator Author

ferd commented Feb 21, 2020

Managed to get an OTP-18 version going... the problem is that I use maps:update_with and that didn't exist back then... I'm not excited by the prospect of reimplementing it for fun, but I guess that's what it takes.

- Can't deal with := in map type declarations
- maps:update_with didn't exist back then so we reinvent it for the few
  months until we deprecate OTP 18
max-au and others added 2 commits March 1, 2020 20:53
Reduces dependency verification time for large codebases.
rebar_compiler_dag: implement single-pass pruning
@ferd
Copy link
Collaborator Author

ferd commented Mar 2, 2020

FWIW I've added a contribution from @max-au to this branch which optimizes the DAG purging further and improves incremental build performance, and includes a short fix we found to prevent dangling edges in prior builds from triggering newer rebuilds that no longer have these dependencies.

src/rebar_compiler_dag.erl Outdated Show resolved Hide resolved
THat was code written on a bus and rarely executed, but still the
function called didn't exist for real.
@ferd ferd merged commit be7ad3c into erlang:master Mar 8, 2020
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.

3 participants