Overhaul environment expansion for better performance

The introduction of setup_env as a global concept caused the rebar_port_compiler
implementation to start getting called a LOT. The expansion of environment variables
that happens in the port compiler was O(n^n), which means you could see upwards of
80k invocations of lists:foldl on a single app "./rebar clean". This commit reworks
the expansion to be O(n^2), and reduces the running time for the same operation by
60%+. On a large project like Riak, the end result is that a build went from 200
seconds to 73.

src/rebar_port_compiler.erl

@@ -290,38 +290,51 @@ merge_each_var([{Key, Value} | Rest], Vars) ->
 %% for every other key until no further expansions are possible.
 %%
 expand_vars_loop(Vars) ->
-    expand_vars_loop(Vars, 10).
+    expand_vars_loop(Vars, [], dict:from_list(Vars), 10).
 
-expand_vars_loop(_, 0) ->
+expand_vars_loop(_Pending, _Recurse, _Vars, 0) ->
     ?ABORT("Max. expansion reached for ENV vars!\n", []);
-expand_vars_loop(Vars0, Count) ->
+expand_vars_loop([], [], Vars, _Count) ->
-    Vars = lists:foldl(fun({Key, Value}, Acc) ->
+    lists:keysort(1, dict:to_list(Vars));
-                               expand_vars(Key, Value, Acc)
+expand_vars_loop([], Recurse, Vars, Count) ->
-                       end,
+    expand_vars_loop(Recurse, [], Vars, Count-1);
-                       Vars0, Vars0),
+expand_vars_loop([{K, V} | Rest], Recurse, Vars, Count) ->
-    case orddict:from_list(Vars) of
+    %% Identify the variables that need expansion in this value
-        Vars0 ->
+    case re:run(V, "\\\${?(\\w+)}?", [global, {capture, all_but_first, list}]) of
-            Vars0;
+        {match, Matches} ->
-        _ ->
+            %% Identify the unique variables that need to be expanded
-            expand_vars_loop(Vars, Count-1)
+            UniqueMatches = lists:usort([M || [M] <- Matches]),
+
+            %% For each variable, expand it and return the final value. Note that
+            %% if we have a bunch of unresolvable variables, nothing happens and
+            %% we don't bother attempting further expansion
+            case expand_keys_in_value(UniqueMatches, V, Vars) of
+                V ->
+                    %% No change after expansion; move along
+                    expand_vars_loop(Rest, Recurse, Vars, Count);
+                Expanded ->
+                    %% Some expansion occurred; move to next k/v but revist
+                    %% this value in the next loop to check for further expansion
+                    NewVars = dict:store(K, Expanded, Vars),
+                    expand_vars_loop(Rest, [{K, Expanded} | Recurse], NewVars, Count)
+            end;
+
+        nomatch ->
+            %% No values in this variable need expansion; move along
+            expand_vars_loop(Rest, Recurse, Vars, Count)
     end.
 
-%%
+expand_keys_in_value([], Value, _Vars) ->
-%% Expand all OTHER references to a given K/V pair
+    Value;
-%%
+expand_keys_in_value([Key | Rest], Value, Vars) ->
-expand_vars(Key, Value, Vars) ->
+    NewValue = case dict:find(Key, Vars) of
-    lists:foldl(
+                   {ok, KValue} ->
-      fun({AKey, AValue}, Acc) ->
+                       rebar_utils:expand_env_variable(Value, Key, KValue);
-              NewValue = case AKey of
+                   error ->
-                             Key ->
+                       Value
-                                 AValue;
+               end,
-                             _ ->
+    expand_keys_in_value(Rest, NewValue, Vars).
-                                 rebar_utils:expand_env_variable(AValue,
+
-                                                                 Key, Value)
-                         end,
-              [{AKey, NewValue} | Acc]
-      end,
-      [], Vars).
 
 expand_command(TmplName, Env, InFiles, OutFile) ->
     Cmd0 = proplists:get_value(TmplName, Env),

src/rebar_utils.erl

@@ -186,12 +186,18 @@ expand_code_path() ->
 %% The end of form $FOO is delimited with whitespace or eol
 %%
 expand_env_variable(InStr, VarName, RawVarValue) ->
-    ReOpts = [global, {return, list}],
+    case string:chr(InStr, $$) of
-    VarValue = re:replace(RawVarValue, "\\\\", "\\\\\\\\", ReOpts),
+        0 ->
-    R1 = re:replace(InStr, "\\\$" ++ VarName ++ "\\s", VarValue ++ " ",
+            %% No variables to expand
-                    [global]),
+            InStr;
-    R2 = re:replace(R1, "\\\$" ++ VarName ++ "\$", VarValue),
+        _ ->
-    re:replace(R2, "\\\${" ++ VarName ++ "}", VarValue, ReOpts).
+            ReOpts = [global, {return, list}],
+            VarValue = re:replace(RawVarValue, "\\\\", "\\\\\\\\", [global]),
+            %% Use a regex to match/replace:
+            %% $FOO\s | ${FOO} | $FOO eol
+            RegEx = io_lib:format("\\\$(~s(\\s|$)|{~s})", [VarName, VarName]),
+            re:replace(InStr, RegEx, VarValue ++ "\\2", ReOpts)
+    end.
 
 
 %% ====================================================================