File 2341-Produce-less-garbage-on-module-lookup.patch of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 2341-Produce-less-garbage-on-module-lookup.patch of Package erlang

From 73fa7599de1ba1bdac9c46a30c7c60ff286bd55c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@dashbit.co>
Date: Thu, 25 Jan 2024 13:33:57 +0100
Subject: [PATCH] Produce less garbage on module lookup

The previous implementation would always build
(and reverse) the code paths whenever looking
up a module, even when all paths were cached.

The new implementation uses a map with unique
integers and we only update the cache when
it is stale.
---
 lib/kernel/src/code_server.erl | 225 +++++++++++++++++----------------
 1 file changed, 119 insertions(+), 106 deletions(-)

diff --git a/lib/kernel/src/code_server.erl b/lib/kernel/src/code_server.erl
index 5315fa1e1a..b6f0d34c3a 100644
--- a/lib/kernel/src/code_server.erl
+++ b/lib/kernel/src/code_server.erl
@@ -43,7 +43,8 @@
 
 -record(state, {supervisor :: pid(),
 		root :: file:name_all(),
-		path :: [{file:name_all(), cache | nocache}],
+		path :: [{file:name_all(), nocache | integer()}],
+                path_cache = #{} :: #{integer() => #{string() => []}},
 		moddb :: ets:table(),
 		namedb :: ets:table(),
 		on_load = [] :: [on_load_item()],
@@ -265,43 +266,41 @@ handle_call({dir,Dir}, _From, S) ->
     Resp = do_dir(Root,Dir,S#state.namedb),
     {reply,Resp,S};
 
-handle_call({add_path,Where,Dir0,Cache}, _From,
-	    #state{namedb=Namedb,path=Path0}=S) ->
-    {Resp,Path} = add_path(Where, Dir0, Path0, Cache, Namedb),
-    {reply,Resp,S#state{path=Path}};
+handle_call({add_path,Where,Dir0,Control}, _From,
+	    #state{namedb=Namedb,path=Path0,path_cache=Cache0}=S) ->
+    {Resp,Path,Cache} = add_path(Where, Dir0, Path0, Control, Cache0, Namedb),
+    {reply,Resp,S#state{path=Path,path_cache=Cache}};
 
-handle_call({add_paths,Where,Dirs0,Cache}, _From,
-	    #state{namedb=Namedb,path=Path0}=S) ->
-    {Resp,Path} = add_paths(Where, Dirs0, Path0, Cache, Namedb),
-    {reply,Resp,S#state{path=Path}};
+handle_call({add_paths,Where,Dirs0,Control}, _From,
+	    #state{namedb=Namedb,path=Path0,path_cache=Cache0}=S) ->
+    {Resp,Path,Cache} = add_paths(Where, Dirs0, Path0, Control, Cache0, Namedb),
+    {reply,Resp,S#state{path=Path,path_cache=Cache}};
 
-handle_call({set_path,PathList,Cache}, _From,
-	    #state{root=Root,path=Path0,namedb=Namedb}=S) ->
-    {Resp,Path,NewDb} = set_path(PathList, Path0, Cache, Namedb, Root),
-    {reply,Resp,S#state{path=Path,namedb=NewDb}};
+handle_call({set_path,PathList,Control}, _From,
+	    #state{namedb=Namedb,root=Root,path=Path0}=S) ->
+    {Resp,Path,NewDb} = set_path(PathList, Path0, Control, Namedb, Root),
+    {reply,Resp,S#state{namedb=NewDb,path=Path,path_cache=#{}}};
 
 handle_call({del_path,Name}, _From,
-	    #state{path=Path0,namedb=Namedb}=S) ->
-    {Resp,Path} = del_path(Name, Path0, Namedb),
-    {reply,Resp,S#state{path=Path}};
+	    #state{namedb=Namedb,path=Path0,path_cache=Cache0}=S) ->
+    {Resp,Path,Cache} = del_path(Name, Path0, Cache0, Namedb),
+    {reply,Resp,S#state{path=Path,path_cache=Cache}};
 
 handle_call({del_paths,Names}, _From,
-            #state{path=Path0,namedb=Namedb}=S) ->
-    {Resp,Path} = del_paths(Names, Path0, Namedb),
-    {reply,Resp,S#state{path=Path}};
+            #state{namedb=Namedb,path=Path0,path_cache=Cache0}=S) ->
+    {Resp,Path,Cache} = del_paths(Names, Path0, Cache0, Namedb),
+    {reply,Resp,S#state{path=Path,path_cache=Cache}};
 
-handle_call({replace_path,Name,Dir,Cache}, _From,
-	    #state{path=Path0,namedb=Namedb}=S) ->
-    {Resp,Path} = replace_path(Name, Dir, Path0, Cache, Namedb),
-    {reply,Resp,S#state{path=Path}};
+handle_call({replace_path,Name,Dir,Control}, _From,
+	    #state{namedb=Namedb,path=Path0,path_cache=Cache0}=S) ->
+    {Resp,Path,Cache} = replace_path(Name, Dir, Path0, Control, Cache0, Namedb),
+    {reply,Resp,S#state{path=Path,path_cache=Cache}};
 
 handle_call(get_path, _From, S) ->
     {reply,[P || {P, _Cache} <- S#state.path],S};
 
 handle_call(clear_cache, _From, S) ->
-    Path = [{P, if is_atom(Cache) -> Cache; true -> cache end} ||
-            {P, Cache} <- S#state.path],
-    {reply,ok,S#state{path=Path}};
+    {reply,ok,S#state{path_cache=#{}}};
 
 handle_call({load_module,PC,Mod,File,Purge,EnsureLoaded}, From, S)
   when is_atom(Mod) ->
@@ -532,8 +531,11 @@ cache_path(Path) ->
     Default = cache_boot_paths(),
     [{P, do_cache_path(P, Default)} || P <- Path].
 
-do_cache_path(".", _) -> nocache;
-do_cache_path(_, Default) -> Default.
+do_cache_path(".", _Default) -> nocache;
+do_cache_path(_Path, Default) -> cache_key(Default).
+
+cache_key(nocache) -> nocache;
+cache_key(cache) -> erlang:unique_integer().
 
 cache_boot_paths() ->
     case init:get_argument(cache_boot_paths) of
@@ -597,12 +599,12 @@ merge_path1(_,IPath,Acc) ->
     lists:reverse(Acc) ++ IPath.
 
 add_pa_pz(Path0, Patha, Pathz) ->
-    {_,Path1} = add_paths(first,Patha,Path0,nocache,false),
+    {_,Path1,_Cache1} = add_paths(first,Patha,Path0,nocache,#{},false),
     case Pathz of
         [] ->
             Path1;
         _ ->
-            {_,Path2} = add_paths(first,Pathz,lists:reverse(Path1),nocache,false),
+            {_,Path2,_Cache2} = add_paths(first,Pathz,lists:reverse(Path1),nocache,#{},false),
             lists:reverse(Path2)
     end.
 
@@ -718,16 +720,16 @@ do_check_path([Dir | Tail], PathChoice, ArchiveExt, Acc) ->
 %%
 %% Add new path(s).
 %%
-add_path(Where,Dir0,Path,Cache,NameDb) when is_list(Dir0) ->
+add_path(Where,Dir0,Path,Control,Cache,NameDb) when is_list(Dir0) ->
     Dir = filename:join([Dir0]), % Normalize
     case check_path([Dir]) of
 	{ok, [NewDir]} ->
-	    {true, do_add(Where,NewDir,Path,Cache,NameDb)};
+	    do_add(Where,NewDir,Path,Control,Cache,NameDb);
 	Error ->
-	    {Error, Path}
+	    {Error, Path, Cache}
     end;
-add_path(_,_,Path,_,_) ->
-    {{error, bad_directory}, Path}.
+add_path(_,_,Path,_,Cache,_) ->
+    {{error, bad_directory}, Path, Cache}.
 
 %%
 %% If the new directory is added first or if the directory didn't exist
@@ -735,16 +737,26 @@ add_path(_,_,Path,_,_) ->
 %% If NameDb is false we should NOT update NameDb as it is done later
 %% then the table is created :-)
 %%
-do_add(first,Dir,Path,Cache,NameDb) ->
+do_add(first,Dir,Path0,Control,Cache,NameDb) ->
     update(Dir, NameDb),
-    [{Dir, Cache}|lists:keydelete(Dir,1,Path)];
-do_add(last,Dir,Path,Cache,NameDb) ->
-    case lists:keymember(Dir,1,Path) of
-	true ->
-	    lists:keyreplace(Dir,1,Path,{Dir,Cache});
+    Pair = {Dir, cache_key(Control)},
+
+    case lists:keytake(Dir,1,Path0) of
+        false ->
+            {true, [Pair|Path0], Cache};
+
+        {value, {_Dir, CacheKey}, Path1} ->
+            {true, [Pair|Path1], maps:remove(CacheKey, Cache)}
+    end;
+do_add(last,Dir,Path,Control,Cache,NameDb) ->
+    Pair = {Dir, cache_key(Control)},
+
+    case lists:keyfind(Dir,1,Path) of
+	{_Dir, CacheKey} ->
+	    {true, lists:keyreplace(Dir,1,Path,Pair), maps:remove(CacheKey, Cache)};
 	false ->
 	    maybe_update(Dir, NameDb),
-	    Path ++ [{Dir,Cache}]
+	    {true, Path ++ [Pair], Cache}
     end.
 
 %% Do not update if the same name already exists !
@@ -759,11 +771,11 @@ update(Dir, NameDb) ->
 %%
 %% Set a completely new path.
 %%
-set_path(NewPath, OldPath, Cache, NameDb, Root) ->
+set_path(NewPath, OldPath, Control, NameDb, Root) ->
     case check_path(NewPath) of
 	{ok, NewPath2} ->
 	    ets:delete(NameDb),
-            NewPath3 = [{P, Cache} || P <- NewPath2],
+            NewPath3 = [{P, cache_key(Control)} || P <- NewPath2],
 	    NewDb = create_namedb(NewPath3, Root),
 	    {true, NewPath3, NewDb};
 	Error ->
@@ -859,36 +871,34 @@ try_archive_subdirs(_Archive, Base, []) ->
 %% Name can be either the name in .../Name[-*] or
 %% the complete directory name.
 %%
-del_path(Name0,Path,NameDb) ->
+del_path(Name0,Path,Cache,NameDb) ->
     case catch filename:join([to_list(Name0)]) of
 	{'EXIT',_} ->
 	    {{error,bad_name},Path};
 	Name ->
-	    case del_path1(Name,Path,NameDb) of
-		Path -> % Nothing has changed
-		    {false,Path};
-		NewPath ->
-		    {true,NewPath}
-	    end
+	    case del_path1(Name,Path,Cache,NameDb,[]) of
+                {NPath, NCache} -> {true, NPath, NCache};
+                false -> {false, Path, Cache}
+            end
     end.
 
-del_path1(Name,[{P, Cache}|Path],NameDb) ->
+del_path1(Name,[{P, CacheKey}|Path],Cache,NameDb,Acc) ->
     case get_name(P) of
 	Name ->
 	    delete_name(Name, NameDb),
 	    insert_old_shadowed(Name, Path, NameDb),
-	    Path;
+	    {lists:reverse(Acc, Path), maps:remove(CacheKey, Cache)};
 	_ when Name =:= P ->
 	    case delete_name_dir(Name, NameDb) of
 		true -> insert_old_shadowed(get_name(Name), Path, NameDb);
 		false -> ok
 	    end,
-	    Path;
+            {lists:reverse(Acc, Path), maps:remove(CacheKey, Cache)};
 	_ ->
-	    [{P, Cache}|del_path1(Name,Path,NameDb)]
+            del_path1(Name, Path, Cache, NameDb, [{P, CacheKey} | Acc])
     end;
-del_path1(_,[],_) ->
-    [].
+del_path1(_,[],_,_,_) ->
+    false.
 
 insert_old_shadowed(Name, [{P, _Cache}|Path], NameDb) ->
     case get_name(P) of
@@ -902,27 +912,30 @@ insert_old_shadowed(_, [], _) ->
 %% Replace an old occurrence of an directory with name .../Name[-*].
 %% If it does not exist, put the new directory last in Path.
 %%
-replace_path(Name,Dir,Path,Cache,NameDb) ->
+replace_path(Name,Dir,Path,Control,Cache,NameDb) ->
     case catch check_pars(Name,Dir) of
 	{ok,N,D} ->
-	    {true,replace_path1(N,D,Path,Cache,NameDb)};
+	    replace_path1(N,D,Path,Control,Cache,NameDb,[]);
 	{'EXIT',_} ->
-	    {{error,{badarg,[Name,Dir]}},Path};
+	    {{error,{badarg,[Name,Dir]}},Path,Cache};
 	Error ->
-	    {Error,Path}
+	    {Error,Path,Cache}
     end.
 
-replace_path1(Name,Dir,[{P, _}=Pair|Path],Cache,NameDb) ->
+replace_path1(Name,Dir,[{P, CacheKey}|Path],Control,Cache,NameDb,Acc) ->
     case get_name(P) of
 	Name ->
 	    insert_name(Name, Dir, NameDb),
-	    [{Dir, Cache}|Path];
+            NPath = lists:reverse(Acc, [{Dir, cache_key(Control)}|Path]),
+	    {true, NPath, maps:remove(CacheKey, Cache)};
 	_ ->
-	    [Pair|replace_path1(Name,Dir,Path,Cache,NameDb)]
+            NAcc = [{P, CacheKey}|Acc],
+	    replace_path1(Name,Dir,Path,Control,Cache,NameDb,NAcc)
     end;
-replace_path1(Name, Dir, [], Cache, NameDb) ->
+replace_path1(Name,Dir,[],Control,Cache,NameDb,Acc) ->
     insert_name(Name, Dir, NameDb),
-    [{Dir, Cache}].
+    NPath = lists:reverse(Acc, [{Dir, cache_key(Control)}]),
+    {true, NPath, Cache}.
 
 check_pars(Name,Dir) ->
     N = to_list(Name),
@@ -1074,17 +1087,17 @@ get_mods([], _) -> [].
 is_sticky(Mod, Db) ->
     erlang:module_loaded(Mod) andalso (ets:lookup(Db, {sticky, Mod}) =/= []).
 
-add_paths(Where,[Dir|Tail],Path,Cache,NameDb) ->
-    {_,NPath} = add_path(Where,Dir,Path,Cache,NameDb),
-    add_paths(Where,Tail,NPath,Cache,NameDb);
-add_paths(_,_,Path,_,_) ->
-    {ok,Path}.
+add_paths(Where,[Dir|Tail],Path,Control,Cache,NameDb) ->
+    {_,NPath,NCache} = add_path(Where,Dir,Path,Control,Cache,NameDb),
+    add_paths(Where,Tail,NPath,Control,NCache,NameDb);
+add_paths(_,_,Path,_,Cache,_) ->
+    {ok,Path,Cache}.
 
-del_paths([Name | Names],Path,NameDb) ->
-    {_,NPath} = del_path(Name, Path, NameDb),
-    del_paths(Names,NPath,NameDb);
-del_paths(_,Path,_) ->
-    {ok,Path}.
+del_paths([Name | Names],Path,Cache,NameDb) ->
+    {_,NPath,NCache} = del_path(Name, Path, Cache, NameDb),
+    del_paths(Names,NPath,NCache,NameDb);
+del_paths(_,Path,Cache,_) ->
+    {ok,Path,Cache}.
 
 try_finish_module(File, Mod, PC, EnsureLoaded, From, St) ->
     Action =  fun(_, S) ->
@@ -1130,16 +1143,16 @@ try_finish_module_2(File, Mod, PC, From, EnsureLoaded, St0) ->
     end,
     handle_on_load(Res, Action, Mod, From, St0).
 
-get_object_code(#state{path=Path} = St, Mod) when is_atom(Mod) ->
+get_object_code(#state{path=Path,path_cache=Cache} = St, Mod) when is_atom(Mod) ->
     ModStr = atom_to_list(Mod),
     case erl_prim_loader:is_basename(ModStr) of
         true ->
-            case mod_to_bin(Path, ModStr ++ objfile_extension(), []) of
-                {Binary, File, NewPath} ->
-                    {Binary, File, St#state{path=NewPath}};
+            case mod_to_bin(Path, ModStr ++ objfile_extension(), Cache) of
+                {Binary, File, NCache} ->
+                    {Binary, File, St#state{path_cache=NCache}};
 
-                {error, NewPath} ->
-                    {error, St#state{path=NewPath}}
+                {error, NCache} ->
+                    {error, St#state{path_cache=NCache}}
             end;
 
         false ->
@@ -1199,42 +1212,42 @@ loader_down(#state{loading = Loading0} = St, {Mod, Bin, FName}) ->
             St
     end.
 
-mod_to_bin([{Dir, nocache}|Tail], ModFile, Acc) ->
+mod_to_bin([{Dir, nocache}|Tail], ModFile, Cache) ->
     File = filename:append(Dir, ModFile),
 
     case erl_prim_loader:read_file(File) of
-        error ->
-            mod_to_bin(Tail, ModFile, [{Dir, nocache} | Acc]);
-
-        {ok,Bin} ->
-            Path = lists:reverse(Acc, [{Dir, nocache} | Tail]),
-            {Bin, absname_when_relative(File), Path}
+        error -> mod_to_bin(Tail, ModFile, Cache);
+        {ok,Bin} -> {Bin, absname_when_relative(File), Cache}
     end;
-mod_to_bin([{Dir, Cache0}|Tail], ModFile, Acc) ->
-    case with_cache(Cache0, Dir, ModFile) of
+mod_to_bin([{Dir, CacheKey}|Tail], ModFile, Cache) when is_integer(CacheKey) ->
+    case with_cache(CacheKey, Dir, ModFile, Cache) of
         {true, Cache1} ->
             File = filename:append(Dir, ModFile),
-            Path = lists:reverse(Acc, [{Dir, Cache1} | Tail]),
-            {missing, absname_when_relative(File), Path};
+            {missing, absname_when_relative(File), Cache1};
         {false, Cache1} ->
-            mod_to_bin(Tail, ModFile, [{Dir, Cache1} | Acc])
+            mod_to_bin(Tail, ModFile, Cache1)
     end;
-mod_to_bin([], ModFile, Acc) ->
+mod_to_bin([], ModFile, Cache) ->
     %% At last, try also erl_prim_loader's own method
     case erl_prim_loader:get_file(ModFile) of
-        error ->
-            {error, lists:reverse(Acc)};     % No more alternatives !
-        {ok,Bin,FName} ->
-            {Bin, absname(FName), lists:reverse(Acc)}
+        error -> {error, Cache};
+        {ok,Bin,FName} -> {Bin, absname(FName), Cache}
     end.
 
-with_cache(cache, Dir, ModFile) ->
-    case erl_prim_loader:list_dir(Dir) of
-        {ok, Entries} -> with_cache(maps:from_keys(Entries, []), Dir, ModFile);
-        error -> {false, cache}
-    end;
-with_cache(Cache, _Dir, ModFile) when is_map(Cache) ->
-    {is_map_key(ModFile, Cache), Cache}.
+with_cache(CacheKey, Dir, ModFile, Cache) ->
+    case Cache of
+        #{CacheKey := Set} ->
+            {is_map_key(ModFile, Set), Cache};
+
+        #{} ->
+            case erl_prim_loader:list_dir(Dir) of
+                {ok, Entries} ->
+                    Set = maps:from_keys(Entries, []),
+                    {is_map_key(ModFile, Set), maps:put(CacheKey, Set, Cache)};
+                error ->
+                    {false, Cache}
+            end
+    end.
 
 absname_when_relative(File) ->
     case filename:pathtype(File) of
-- 
2.35.3

Places

File 2341-Produce-less-garbage-on-module-lookup.patch of Package erlang

Places