File 2461-Skip-literals-on-v3_core-and-sys_core_alia... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 2461-Skip-literals-on-v3_core-and-sys_core_alias.patch of Package erlang (Revision a7029ee97b719a45764ae3b135bbdda2)

Currently displaying revision a7029ee97b719a45764ae3b135bbdda2 , Show latest

From 34e4f91842e842214769a133efffba3b746a7bf2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@dashbit.co>
Date: Thu, 29 Oct 2020 13:34:39 +0100
Subject: [PATCH] Skip literals on v3_core and sys_core_alias

When compiling code with large amount of literals,
there was a performance degradation between Erlang/OTP
22.3 and 23.1.

For example, the gisted code below takes 10 extra seconds:

https://gist.github.com/josevalim/694c1799143fcf25e43aa27e3e11e4c1

In particular, the v3_core pass went from 1.8s to 3.2s.
When profiling the pass, we could see a lot of time was
spent on the new lowering pass in v3_core. In particular,
cerl_trees:mapfold/3 would show up in profilers:

cerl_trees:mapfold/4   3220377    19.14   2447684  [0.76]

Which is quite curious as the linked code does not trigger
any of the lowering cases.

Under further analysis, this happened because cerl_trees would
traverse literals and other constructs, which is a waste given
there is nothing to lower in many of them.

This commit speeds up both v3_core since cerl_trees is invoked
33% less frequently and the pass is overall 20% faster.
sys_core_alias also got 33% faster. The same technique may be
applicable to other passes.
---
 lib/compiler/src/cerl_trees.erl     | 226 +++++++++++++++-------------
 lib/compiler/src/sys_core_alias.erl |  27 ++--
 lib/compiler/src/v3_core.erl        |  13 +-
 3 files changed, 146 insertions(+), 120 deletions(-)

diff --git a/lib/compiler/src/cerl_trees.erl b/lib/compiler/src/cerl_trees.erl
index a2089b5c1b..383d9b5214 100644
--- a/lib/compiler/src/cerl_trees.erl
+++ b/lib/compiler/src/cerl_trees.erl
@@ -352,131 +352,141 @@ mapfold(F, S0, T) ->
 
 
 %% @spec mapfold(Pre, Post, Initial::term(), Tree::cerl()) -> {cerl(), term()}
-%%       Pre  = (cerl(), term()) -> {cerl(), term()}
+%%       Pre  = (cerl(), term()) -> {cerl(), term()} | skip
 %%       Post = (cerl(), term()) -> {cerl(), term()}
 %%
 %% @doc Does a combined map/fold operation on the nodes of the
 %% tree. It begins by calling <code>Pre</code> on the tree, using the
-%% <code>Initial</code> value. It then deconstructs the top node of
+%% <code>Initial</code> value. <code>Pre</code> must either return a
+%% tree with an updated accumulator or the atom <code>skip</code>.
+%%
+%% If a tree is returned, this function deconstructs the top node of
 %% the returned tree and recurses on the children, using the returned
 %% value as the new initial and carrying the returned values from one
 %% call to the next. Finally it reassembles the top node from the
 %% children, calls <code>Post</code> on it and returns the result.
+%%
+%% If <code>skip</code> is returned, it returns the tree and accumulator
+%% as is.
 
--spec mapfold(fun((cerl:cerl(), term()) -> {cerl:cerl(), term()}),
+-spec mapfold(fun((cerl:cerl(), term()) -> {cerl:cerl(), term()} | skip),
               fun((cerl:cerl(), term()) -> {cerl:cerl(), term()}),
 	      term(), cerl:cerl()) -> {cerl:cerl(), term()}.
 
 mapfold(Pre, Post, S00, T0) ->
-    {T, S0} = Pre(T0, S00),
-    case type(T) of
- 	literal ->
-	    case concrete(T) of
-		[_ | _] ->
+    case Pre(T0, S00) of
+	{T, S0} ->
+	    case type(T) of
+		literal ->
+		    case concrete(T) of
+			[_ | _] ->
+			    {T1, S1} = mapfold(Pre, Post, S0, cons_hd(T)),
+			    {T2, S2} = mapfold(Pre, Post, S1, cons_tl(T)),
+			    Post(update_c_cons(T, T1, T2), S2);
+			V when tuple_size(V) > 0 ->
+			    {Ts, S1} = mapfold_list(Pre, Post, S0, tuple_es(T)),
+			    Post(update_c_tuple(T, Ts), S1);
+			_ ->
+			    Post(T, S0)
+		    end;
+		var ->
+		    Post(T, S0);
+		values ->
+		    {Ts, S1} = mapfold_list(Pre, Post, S0, values_es(T)),
+		    Post(update_c_values(T, Ts), S1);
+		cons ->
 		    {T1, S1} = mapfold(Pre, Post, S0, cons_hd(T)),
 		    {T2, S2} = mapfold(Pre, Post, S1, cons_tl(T)),
-		    Post(update_c_cons(T, T1, T2), S2);
-		V when tuple_size(V) > 0 ->
+		    Post(update_c_cons_skel(T, T1, T2), S2);
+		tuple ->
 		    {Ts, S1} = mapfold_list(Pre, Post, S0, tuple_es(T)),
-		    Post(update_c_tuple(T, Ts), S1);
-		_ ->
-		    Post(T, S0)
+		    Post(update_c_tuple_skel(T, Ts), S1);
+		map ->
+		    {M , S1} = mapfold(Pre, Post, S0, map_arg(T)),
+		    {Ts, S2} = mapfold_list(Pre, Post, S1, map_es(T)),
+		    Post(update_c_map(T, M, Ts), S2);
+		map_pair ->
+		    {Op,  S1} = mapfold(Pre, Post, S0, map_pair_op(T)),
+		    {Key, S2} = mapfold(Pre, Post, S1, map_pair_key(T)),
+		    {Val, S3} = mapfold(Pre, Post, S2, map_pair_val(T)),
+		    Post(update_c_map_pair(T,Op,Key,Val), S3);
+		'let' ->
+		    {Vs, S1} = mapfold_list(Pre, Post, S0, let_vars(T)),
+		    {A, S2} = mapfold(Pre, Post, S1, let_arg(T)),
+		    {B, S3} = mapfold(Pre, Post, S2, let_body(T)),
+		    Post(update_c_let(T, Vs, A, B), S3);
+		seq ->
+		    {A, S1} = mapfold(Pre, Post, S0, seq_arg(T)),
+		    {B, S2} = mapfold(Pre, Post, S1, seq_body(T)),
+		    Post(update_c_seq(T, A, B), S2);
+		apply ->
+		    {E, S1} = mapfold(Pre, Post, S0, apply_op(T)),
+		    {As, S2} = mapfold_list(Pre, Post, S1, apply_args(T)),
+		    Post(update_c_apply(T, E, As), S2);
+		call ->
+		    {M, S1} = mapfold(Pre, Post, S0, call_module(T)),
+		    {N, S2} = mapfold(Pre, Post, S1, call_name(T)),
+		    {As, S3} = mapfold_list(Pre, Post, S2, call_args(T)),
+		    Post(update_c_call(T, M, N, As), S3);
+		primop ->
+		    {N, S1} = mapfold(Pre, Post, S0, primop_name(T)),
+		    {As, S2} = mapfold_list(Pre, Post, S1, primop_args(T)),
+		    Post(update_c_primop(T, N, As), S2);
+		'case' ->
+		    {A, S1} = mapfold(Pre, Post, S0, case_arg(T)),
+		    {Cs, S2} = mapfold_list(Pre, Post, S1, case_clauses(T)),
+		    Post(update_c_case(T, A, Cs), S2);
+		clause ->
+		    {Ps, S1} = mapfold_list(Pre, Post, S0, clause_pats(T)),
+		    {G, S2} = mapfold(Pre, Post, S1, clause_guard(T)),
+		    {B, S3} = mapfold(Pre, Post, S2, clause_body(T)),
+		    Post(update_c_clause(T, Ps, G, B), S3);
+		alias ->
+		    {V, S1} = mapfold(Pre, Post, S0, alias_var(T)),
+		    {P, S2} = mapfold(Pre, Post, S1, alias_pat(T)),
+		    Post(update_c_alias(T, V, P), S2);
+		'fun' ->
+		    {Vs, S1} = mapfold_list(Pre, Post, S0, fun_vars(T)),
+		    {B, S2} = mapfold(Pre, Post, S1, fun_body(T)),
+		    Post(update_c_fun(T, Vs, B), S2);
+		'receive' ->
+		    {Cs, S1} = mapfold_list(Pre, Post, S0, receive_clauses(T)),
+		    {E, S2} = mapfold(Pre, Post, S1, receive_timeout(T)),
+		    {A, S3} = mapfold(Pre, Post, S2, receive_action(T)),
+		    Post(update_c_receive(T, Cs, E, A), S3);
+		'try' ->
+		    {E, S1} = mapfold(Pre, Post, S0, try_arg(T)),
+		    {Vs, S2} = mapfold_list(Pre, Post, S1, try_vars(T)),
+		    {B, S3} = mapfold(Pre, Post, S2, try_body(T)),
+		    {Evs, S4} = mapfold_list(Pre, Post, S3, try_evars(T)),
+		    {H, S5} = mapfold(Pre, Post, S4, try_handler(T)),
+		    Post(update_c_try(T, E, Vs, B, Evs, H), S5);
+		'catch' ->
+		    {B, S1} = mapfold(Pre, Post, S0, catch_body(T)),
+		    Post(update_c_catch(T, B), S1);
+		binary ->
+		    {Ds, S1} = mapfold_list(Pre, Post, S0, binary_segments(T)),
+		    Post(update_c_binary(T, Ds), S1);
+		bitstr ->
+		    {Val, S1} = mapfold(Pre, Post, S0, bitstr_val(T)),
+		    {Size, S2} = mapfold(Pre, Post, S1, bitstr_size(T)),
+		    {Unit, S3} = mapfold(Pre, Post, S2, bitstr_unit(T)),
+		    {Type, S4} = mapfold(Pre, Post, S3, bitstr_type(T)),
+		    {Flags, S5} = mapfold(Pre, Post, S4, bitstr_flags(T)),
+		    Post(update_c_bitstr(T, Val, Size, Unit, Type, Flags), S5);
+		letrec ->
+		    {Ds, S1} = mapfold_pairs(Pre, Post, S0, letrec_defs(T)),
+		    {B, S2} = mapfold(Pre, Post, S1, letrec_body(T)),
+		    Post(update_c_letrec(T, Ds, B), S2);
+		module ->
+		    {N, S1} = mapfold(Pre, Post, S0, module_name(T)),
+		    {Es, S2} = mapfold_list(Pre, Post, S1, module_exports(T)),
+		    {As, S3} = mapfold_pairs(Pre, Post, S2, module_attrs(T)),
+		    {Ds, S4} = mapfold_pairs(Pre, Post, S3, module_defs(T)),
+		    Post(update_c_module(T, N, Es, As, Ds), S4)
 	    end;
- 	var ->
-	    Post(T, S0);
-	values ->
-	    {Ts, S1} = mapfold_list(Pre, Post, S0, values_es(T)),
-	    Post(update_c_values(T, Ts), S1);
-	cons ->
-	    {T1, S1} = mapfold(Pre, Post, S0, cons_hd(T)),
-	    {T2, S2} = mapfold(Pre, Post, S1, cons_tl(T)),
-	    Post(update_c_cons_skel(T, T1, T2), S2);
- 	tuple ->
-	    {Ts, S1} = mapfold_list(Pre, Post, S0, tuple_es(T)),
-	    Post(update_c_tuple_skel(T, Ts), S1);
-	map ->
-	    {M , S1} = mapfold(Pre, Post, S0, map_arg(T)),
-	    {Ts, S2} = mapfold_list(Pre, Post, S1, map_es(T)),
-	    Post(update_c_map(T, M, Ts), S2);
-	map_pair ->
-	    {Op,  S1} = mapfold(Pre, Post, S0, map_pair_op(T)),
-	    {Key, S2} = mapfold(Pre, Post, S1, map_pair_key(T)),
-	    {Val, S3} = mapfold(Pre, Post, S2, map_pair_val(T)),
-	    Post(update_c_map_pair(T,Op,Key,Val), S3);
- 	'let' ->
-	    {Vs, S1} = mapfold_list(Pre, Post, S0, let_vars(T)),
-	    {A, S2} = mapfold(Pre, Post, S1, let_arg(T)),
-	    {B, S3} = mapfold(Pre, Post, S2, let_body(T)),
-	    Post(update_c_let(T, Vs, A, B), S3);
-	seq ->
-	    {A, S1} = mapfold(Pre, Post, S0, seq_arg(T)),
-	    {B, S2} = mapfold(Pre, Post, S1, seq_body(T)),
-	    Post(update_c_seq(T, A, B), S2);
- 	apply ->
-	    {E, S1} = mapfold(Pre, Post, S0, apply_op(T)),
-	    {As, S2} = mapfold_list(Pre, Post, S1, apply_args(T)),
-	    Post(update_c_apply(T, E, As), S2);
- 	call ->
-	    {M, S1} = mapfold(Pre, Post, S0, call_module(T)),
-	    {N, S2} = mapfold(Pre, Post, S1, call_name(T)),
-	    {As, S3} = mapfold_list(Pre, Post, S2, call_args(T)),
-	    Post(update_c_call(T, M, N, As), S3);
- 	primop ->
-	    {N, S1} = mapfold(Pre, Post, S0, primop_name(T)),
-	    {As, S2} = mapfold_list(Pre, Post, S1, primop_args(T)),
-	    Post(update_c_primop(T, N, As), S2);
- 	'case' ->
-	    {A, S1} = mapfold(Pre, Post, S0, case_arg(T)),
-	    {Cs, S2} = mapfold_list(Pre, Post, S1, case_clauses(T)),
-	    Post(update_c_case(T, A, Cs), S2);
- 	clause ->
-	    {Ps, S1} = mapfold_list(Pre, Post, S0, clause_pats(T)),
-	    {G, S2} = mapfold(Pre, Post, S1, clause_guard(T)),
-	    {B, S3} = mapfold(Pre, Post, S2, clause_body(T)),
-	    Post(update_c_clause(T, Ps, G, B), S3);
- 	alias ->
-	    {V, S1} = mapfold(Pre, Post, S0, alias_var(T)),
-	    {P, S2} = mapfold(Pre, Post, S1, alias_pat(T)),
-	    Post(update_c_alias(T, V, P), S2);
- 	'fun' ->
-	    {Vs, S1} = mapfold_list(Pre, Post, S0, fun_vars(T)),
-	    {B, S2} = mapfold(Pre, Post, S1, fun_body(T)),
-	    Post(update_c_fun(T, Vs, B), S2);
- 	'receive' ->
-	    {Cs, S1} = mapfold_list(Pre, Post, S0, receive_clauses(T)),
-	    {E, S2} = mapfold(Pre, Post, S1, receive_timeout(T)),
-	    {A, S3} = mapfold(Pre, Post, S2, receive_action(T)),
-	    Post(update_c_receive(T, Cs, E, A), S3);
- 	'try' ->
-	    {E, S1} = mapfold(Pre, Post, S0, try_arg(T)),
-	    {Vs, S2} = mapfold_list(Pre, Post, S1, try_vars(T)),
-	    {B, S3} = mapfold(Pre, Post, S2, try_body(T)),
-	    {Evs, S4} = mapfold_list(Pre, Post, S3, try_evars(T)),
-	    {H, S5} = mapfold(Pre, Post, S4, try_handler(T)),
-	    Post(update_c_try(T, E, Vs, B, Evs, H), S5);
- 	'catch' ->
-	    {B, S1} = mapfold(Pre, Post, S0, catch_body(T)),
-	    Post(update_c_catch(T, B), S1);
-	binary ->
-	    {Ds, S1} = mapfold_list(Pre, Post, S0, binary_segments(T)),
-	    Post(update_c_binary(T, Ds), S1);
-	bitstr ->
-	    {Val, S1} = mapfold(Pre, Post, S0, bitstr_val(T)),
-	    {Size, S2} = mapfold(Pre, Post, S1, bitstr_size(T)),
-	    {Unit, S3} = mapfold(Pre, Post, S2, bitstr_unit(T)),
-	    {Type, S4} = mapfold(Pre, Post, S3, bitstr_type(T)),
-	    {Flags, S5} = mapfold(Pre, Post, S4, bitstr_flags(T)),
-	    Post(update_c_bitstr(T, Val, Size, Unit, Type, Flags), S5);
-	letrec ->
-	    {Ds, S1} = mapfold_pairs(Pre, Post, S0, letrec_defs(T)),
-	    {B, S2} = mapfold(Pre, Post, S1, letrec_body(T)),
-	    Post(update_c_letrec(T, Ds, B), S2);
-	module ->
-	    {N, S1} = mapfold(Pre, Post, S0, module_name(T)),
-	    {Es, S2} = mapfold_list(Pre, Post, S1, module_exports(T)),
-	    {As, S3} = mapfold_pairs(Pre, Post, S2, module_attrs(T)),
-	    {Ds, S4} = mapfold_pairs(Pre, Post, S3, module_defs(T)),
-	    Post(update_c_module(T, N, Es, As, Ds), S4)
+	skip ->
+	    {T0, S00}
     end.
 
 mapfold_list(Pre, Post, S0, [T | Ts]) ->
diff --git a/lib/compiler/src/sys_core_alias.erl b/lib/compiler/src/sys_core_alias.erl
index 27b13c8ec8..9343999980 100644
--- a/lib/compiler/src/sys_core_alias.erl
+++ b/lib/compiler/src/sys_core_alias.erl
@@ -109,18 +109,23 @@ pre(#c_fun{vars=Vars}=Node, Sub) when ?HAS_SUBS(Sub) ->
     {Node,sub_fold(get_variables(Vars), Sub)};
 
 pre(Node, Sub0) when ?HAS_SUBS(Sub0) ->
-    %% We cache only tuples and cons.
-    case cerl:is_data(Node) andalso not cerl:is_literal(Node) of
-        false ->
-            {Node,Sub0};
+    case cerl:is_literal(Node) of
         true ->
-            Kind = cerl:data_type(Node),
-            Es = cerl:data_es(Node),
-            case sub_cache_nodes(Kind, Es, Sub0) of
-                {Name,Sub1} ->
-                    {cerl:ann_c_var(cerl:get_ann(Node), Name),Sub1};
-                error ->
-                    {Node,Sub0}
+            skip;
+        false ->
+            %% We cache only tuples and cons.
+            case cerl:is_data(Node) of
+                false ->
+                    {Node,Sub0};
+                true ->
+                    Kind = cerl:data_type(Node),
+                    Es = cerl:data_es(Node),
+                    case sub_cache_nodes(Kind, Es, Sub0) of
+                        {Name,Sub1} ->
+                            {cerl:ann_c_var(cerl:get_ann(Node), Name),Sub1};
+                        error ->
+                            {Node,Sub0}
+                    end
             end
     end;
 
diff --git a/lib/compiler/src/v3_core.erl b/lib/compiler/src/v3_core.erl
index 74a45b7279..6ba4bdb251 100644
--- a/lib/compiler/src/v3_core.erl
+++ b/lib/compiler/src/v3_core.erl
@@ -3085,7 +3085,18 @@ c_add_dummy_export(C, [], St) ->
 %%%           -| ['letrec_goto'] )
 
 lbody(B, St) ->
-    cerl_trees:mapfold(fun lexpr/2, St, B).
+    cerl_trees:mapfold(fun skip_lowering/2, fun lexpr/2, St, B).
+
+%% These nodes do not have case or receive within them,
+%% so we can speed up lowering by not traversing them.
+skip_lowering(#c_binary{}, _A) -> skip;
+skip_lowering(#c_call{}, _A) -> skip;
+skip_lowering(#c_cons{}, _A) -> skip;
+skip_lowering(#c_literal{}, _A) -> skip;
+skip_lowering(#c_map{}, _A) -> skip;
+skip_lowering(#c_primop{}, _A) -> skip;
+skip_lowering(#c_tuple{}, _A) -> skip;
+skip_lowering(T, A) -> {T, A}.
 
 lexpr(#c_case{}=Case, St) ->
     %% Split patterns that bind and use the same variable.
-- 
2.26.2

Places

File 2461-Skip-literals-on-v3_core-and-sys_core_alias.patch of Package erlang (Revision a7029ee97b719a45764ae3b135bbdda2)

Places