Optimize path handling

- Only set paths that need to be put as a priority
- Clean up paths before leaving API mode

The first point accounted for some performance cost, but the latter one
explains the 40% overhead in test runs: since rebar3 calls rebar3 a lot
with a bunch of fake apps, and that the new mechanism for path handling
by default does not _remove_ paths, it just _orders_ them, we would end
up in a situation where as the tests ran, more and more fake paths would
get added to the VM.

By the time the run was over, all path handling would take longer since
more paths needed filtering every time. By resetting paths at the end of
an API run, we prevent a given 'project' from polluting another one's
runtime and performance once the API successfully returns.

7 files changed, 53 insertions, 47 deletions

diff --git a/src/rebar3.erl b/src/rebar3.erl
index c931a85..059d530 100644
--- a/src/rebar3.erl
+++ b/src/rebar3.erl
@@ -175,7 +175,20 @@ run_aux(State, RawArgs) ->
 
     State10 = rebar_state:code_paths(State9, default, code:get_path()),
 
-    rebar_core:init_command(rebar_state:command_args(State10, Args), Task).
+    case rebar_core:init_command(rebar_state:command_args(State10, Args), Task) of
+        {ok, State11} ->
+            case rebar_state:get(State11, caller, command_line) of
+                api ->
+                    rebar_paths:unset_paths([deps, plugins], State11),
+                    {ok, State11};
+                _ ->
+                    {ok, State11}
+            end;
+        Other ->
+            Other
+    end.
+
+
 
 %% @doc set up base configuration having to do with verbosity, where
 %% to find config files, and so on, and return an internal rebar3 state term.
diff --git a/src/rebar_hooks.erl b/src/rebar_hooks.erl
index f2ef8a3..358458e 100644
--- a/src/rebar_hooks.erl
+++ b/src/rebar_hooks.erl
@@ -42,9 +42,7 @@ run_provider_hooks_(Dir, Type, Command, Providers, TypeHooks, State) ->
         [] ->
             State;
         HookProviders ->
-            %PluginDepsPaths = lists:usort(rebar_state:code_paths(State, all_plugin_deps)),
-            %code:add_pathsa(PluginDepsPaths),
-            rebar_paths:set_paths([plugins, deps], State),
+            rebar_paths:set_paths([plugins], State),
             Providers1 = rebar_state:providers(State),
             State1 = rebar_state:providers(rebar_state:dir(State, Dir), Providers++Providers1),
             case rebar_core:do(HookProviders, State1) of
@@ -52,8 +50,7 @@ run_provider_hooks_(Dir, Type, Command, Providers, TypeHooks, State) ->
                     ?DEBUG(format_error({bad_provider, Type, Command, ProviderName}), []),
                     throw(?PRV_ERROR({bad_provider, Type, Command, ProviderName}));
                 {ok, State2} ->
-                    %rebar_utils:remove_from_code_path(PluginDepsPaths),
-                    rebar_paths:set_paths([deps, plugins], State2),
+                    rebar_paths:set_paths([deps], State2),
                     State2
             end
     end.
diff --git a/src/rebar_paths.erl b/src/rebar_paths.erl
index 23fc755..900443d 100644
--- a/src/rebar_paths.erl
+++ b/src/rebar_paths.erl
@@ -1,3 +1,5 @@
+%% BEFORE THIS FIX: rebar3 ct  266.78s user 144.06s system 144% cpu 4:33.70 total
+%% CURRENT TIME:    rebar3 ct  419.30s user 301.00s system 152% cpu 7:51.98 total
 -module(rebar_paths).
 -include("rebar.hrl").
 
@@ -8,20 +10,17 @@
 -export([clashing_apps/2]).
 
 -ifdef(TEST).
--export([misloaded_modules/3]).
+-export([misloaded_modules/2]).
 -endif.
 
 -spec set_paths(targets(), rebar_state:t()) -> ok.
 set_paths(UserTargets, State) ->
     Targets = normalize_targets(UserTargets),
     GroupPaths = path_groups(Targets, State),
-    Paths = lists:append([P || {_, P} <- GroupPaths]),
-    [code:del_path(P) || P <- Paths],
-    code:add_pathsa(lists:reverse(Paths)),
-    % set path breaks with escripts; we gotta do it by hand
-    % true = code:set_path(lists:append([P || {_, P} <- GroupPaths])),
+    Paths = lists:append(lists:reverse([P || {_, P} <- GroupPaths])),
+    code:add_pathsa(Paths),
     AppGroups = app_groups(Targets, State),
-    purge_and_load(AppGroups, code:all_loaded(), sets:new()),
+    purge_and_load(AppGroups, sets:new()),
     ok.
 
 -spec unset_paths(targets(), rebar_state:t()) -> ok.
@@ -33,6 +32,7 @@ unset_paths(UserTargets, State) ->
     purge(Paths, code:all_loaded()),
     ok.
 
+-spec clashing_apps(targets(), rebar_state:t()) -> [{target(), [binary()]}].
 clashing_apps(Targets, State) ->
     AppGroups = app_groups(Targets, State),
     AppNames = [{G, sets:from_list(
@@ -66,9 +66,9 @@ normalize_targets(List) ->
     ),
     lists:reverse(TmpList).
 
-purge_and_load([], _, _) ->
+purge_and_load([], _) ->
     ok;
-purge_and_load([{_Group, Apps}|Rest], ModPaths, Seen) ->
+purge_and_load([{_Group, Apps}|Rest], Seen) ->
     %% We have: a list of all applications in the current priority group,
     %% a list of all loaded modules with their active path, and a list of
     %% seen applications.
@@ -79,7 +79,12 @@ purge_and_load([{_Group, Apps}|Rest], ModPaths, Seen) ->
     %% 3. unload and reload apps that may have changed paths in order
     %%    to get updated module lists and specs
     %%    (we ignore started apps and apps that have not run for this)
-    %% 4. create a list of modules to check from that app list
+    %%    This part turns out to be the bottleneck of this module, so
+    %%    to speed it up, using clash detection proves useful:
+    %%    only reload apps that clashed since others are unlikely to
+    %%    conflict in significant ways
+    %% 4. create a list of modules to check from that app list—only loaded
+    %%    modules make sense to check.
     %% 5. check the modules to match their currently loaded paths with
     %%    the path set from the apps in the current group; modules
     %%    that differ must be purged; others can stay
@@ -126,27 +131,29 @@ purge_and_load([{_Group, Apps}|Rest], ModPaths, Seen) ->
          end || App <- GoodApps,
                 AppName <- [binary_to_atom(rebar_app_info:name(App), utf8)]]
     ),
+    ModPaths = [{Mod,Path} || Mod <- CandidateMods,
+                              erlang:function_exported(Mod, module_info, 0),
+                              {file, Path} <- [code:is_loaded(Mod)]],
 
     %% 5)
-    Mods = misloaded_modules(CandidateMods, GoodAppPaths, ModPaths),
+    Mods = misloaded_modules(GoodAppPaths, ModPaths),
     [purge_mod(Mod) || Mod <- Mods],
-    purge_and_load(Rest, ModPaths,
-                   sets:union(Seen, sets:from_list(AppNames))).
+
+    purge_and_load(Rest, sets:union(Seen, sets:from_list(AppNames))).
 
 purge(Paths, ModPaths) ->
     SortedPaths = lists:sort(Paths),
-    lists:map(fun purge_mod/1, lists:usort(
-        [Mod || {Mod, Path} <- ModPaths,
-                is_list(Path), % not 'preloaded' or mocked
-                any_prefix(Path, SortedPaths)]
-    )).
+    lists:map(fun purge_mod/1,
+              [Mod || {Mod, Path} <- ModPaths,
+                      is_list(Path), % not 'preloaded' or mocked
+                      any_prefix(Path, SortedPaths)]
+             ).
 
-misloaded_modules(Mods, GoodAppPaths, ModPaths) ->
+misloaded_modules(GoodAppPaths, ModPaths) ->
     %% Identify paths that are invalid; i.e. app paths that cover an
     %% app in the desired group, but are not in the desired group.
     lists:usort(
-        [Mod || Mod <- Mods,
-                {_, Path} <- [lists:keyfind(Mod, 1, ModPaths)],
+        [Mod || {Mod, Path} <- ModPaths,
                 is_list(Path), % not 'preloaded' or mocked
                 not any_prefix(Path, GoodAppPaths)]
     ).
@@ -157,15 +164,7 @@ any_prefix(Path, Paths) ->
 %% assume paths currently set are good; only unload a module so next call
 %% uses the correctly set paths
 purge_mod(Mod) ->
-    case erlang:check_process_code(self(), Mod) of
-        false ->
-            code:purge(Mod),
-            code:delete(Mod);
-        _ ->
-            %% cannot purge safely without killing ourselves
-            code:soft_purge(Mod) andalso
-            code:delete(Mod)
-    end.
+    code:soft_purge(Mod) andalso code:delete(Mod).
 
 
 %% This is a tricky O(n²) check since we want to
diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl
index a9c7e00..2a78c6e 100644
--- a/src/rebar_plugins.erl
+++ b/src/rebar_plugins.erl
@@ -125,7 +125,7 @@ handle_plugin(Profile, Plugin, State, Upgrade) ->
 
         %% Store plugin code paths so we can remove them when compiling project apps
         State4 = rebar_state:update_code_paths(State3, all_plugin_deps, PreBuiltPaths++NewCodePaths),
-        rebar_paths:set_paths([plugins, deps], State4),
+        rebar_paths:set_paths([plugins], State4),
 
         {plugin_providers(Plugin), State4}
     catch
diff --git a/src/rebar_prv_compile.erl b/src/rebar_prv_compile.erl
index 4b36636..ee96d9f 100644
--- a/src/rebar_prv_compile.erl
+++ b/src/rebar_prv_compile.erl
@@ -37,7 +37,7 @@ init(State) ->
 -spec do(rebar_state:t()) -> {ok, rebar_state:t()} | {error, string()}.
 do(State) ->
     IsDepsOnly = is_deps_only(State),
-    rebar_paths:set_paths([deps, plugins], State),
+    rebar_paths:set_paths([deps], State),
 
     Providers = rebar_state:providers(State),
     Deps = rebar_state:deps_to_build(State),
@@ -50,7 +50,7 @@ do(State) ->
                      handle_project_apps(Providers, State)
              end,
 
-    rebar_paths:set_paths([plugins, deps], State1),
+    rebar_paths:set_paths([plugins], State1),
 
     {ok, State1}.
 
@@ -172,10 +172,10 @@ compile(State, Providers, AppInfo) ->
     %% The rebar_otp_app compilation step is safe regarding the
     %% overall path management, so we can just load all plugins back
     %% in memory.
-    rebar_paths:set_paths([plugins, deps], State),
+    rebar_paths:set_paths([plugins], State),
     AppFileCompileResult = rebar_otp_app:compile(State, AppInfo4),
-    %% Clean up after ourselves, leave things as they were.
-    rebar_paths:set_paths([deps, plugins], State),
+    %% Clean up after ourselves, leave things as they were with deps first
+    rebar_paths:set_paths([deps], State),
 
     case AppFileCompileResult of
         {ok, AppInfo5} ->
@@ -203,7 +203,7 @@ build_app(AppInfo, State) ->
                     %% load plugins since thats where project builders would be
                     rebar_paths:set_paths([plugins, deps], State),
                     Res = Module:build(AppInfo),
-                    rebar_paths:set_paths([deps, plugins], State),
+                    rebar_paths:set_paths([deps], State),
                     case Res of
                         ok -> ok;
                         {error, Reason} -> throw({error, {Module, Reason}})
diff --git a/src/rebar_prv_xref.erl b/src/rebar_prv_xref.erl
index a6b1f73..12063d5 100644
--- a/src/rebar_prv_xref.erl
+++ b/src/rebar_prv_xref.erl
@@ -36,7 +36,7 @@ init(State) ->
 
 -spec do(rebar_state:t()) -> {ok, rebar_state:t()} | {error, string()}.
 do(State) ->
-    rebar_paths:set_paths([deps, plugins], State),
+    rebar_paths:set_paths([deps], State),
     XrefChecks = prepare(State),
     XrefIgnores = rebar_state:get(State, xref_ignores, []),
     %% Run xref checks
@@ -47,7 +47,6 @@ do(State) ->
     QueryChecks = rebar_state:get(State, xref_queries, []),
     QueryResults = lists:foldl(fun check_query/2, [], QueryChecks),
     stopped = xref:stop(xref),
-    rebar_paths:set_paths([plugins, deps], State),
     case XrefResults =:= [] andalso QueryResults =:= [] of
         true ->
             {ok, State};
diff --git a/test/rebar_paths_SUITE.erl b/test/rebar_paths_SUITE.erl
index f2832c0..96cda45 100644
--- a/test/rebar_paths_SUITE.erl
+++ b/test/rebar_paths_SUITE.erl
@@ -212,13 +212,11 @@ check_modules(Config) ->
 
 misloaded_mods(_Config) ->
     Res = rebar_paths:misloaded_modules(
-      [a,b,c,d,e,f],
       ["/1/2/3/4",
        "/1/2/4",
        "/2/1/1",
        "/3/4/5"],
       [{a, "/0/1/2/file.beam"},
-       {aa, "/1/2/3/4/file.beam"},
        {b, "/1/2/3/4/file.beam"},
        {c, "/2/1/file.beam"},
        {f, preloaded},