Reduce memory consumption due to caps handling

SVN Revision: 1711

ChangeLog

@@ -1,3 +1,9 @@
+2008-12-08  Christophe Romain <christophe.romain@process-one.net>
+
+	* src/ejabberd_c2s.erl: Reduce memory consumption due to caps handling
+	* src/mod_pubsub/mod_pubsub.erl: Likewise
+	* src/mod_caps.erl: Likewise
+
 2008-12-08  Mickael Remond  <mremond@process-one.net>
 
 	* src/ejabberd_c2s.erl: Enforce client stanza from attribute
@@ -7,7 +13,7 @@
 2008-12-01  Badlop  <badlop@process-one.net>
 
 	* doc/guide.tex: New subsection Database Connection
-	* doc/guide.html: Like	wise
+	* doc/guide.html: Likewise
 
 2008-12-01  Christophe Romain <christophe.romain@process-one.net>
 

src/ejabberd_c2s.erl

@@ -37,8 +37,7 @@
 	 send_element/2,
 	 socket_type/0,
 	 get_presence/1,
-	 get_subscribed/1,
+	 get_subscribed/1]).
-	 get_subscribed_and_online/1]).
 
 %% gen_fsm callbacks
 -export([init/1,
@@ -84,7 +83,6 @@
 		pres_f = ?SETS:new(),
 		pres_a = ?SETS:new(),
 		pres_i = ?SETS:new(),
-		pres_available = ?DICT:new(),
 		pres_last, pres_pri,
 		pres_timestamp,
 		pres_invis = false,
@@ -212,14 +210,8 @@ init([{SockMod, Socket}, Opts]) ->
     end.
 
 %% Return list of all available resources of contacts,
-%% in form [{JID, Caps}].
 get_subscribed(FsmRef) ->
-    gen_fsm:sync_send_all_state_event(
+    gen_fsm:sync_send_all_state_event(FsmRef, get_subscribed, 1000).
-      FsmRef, get_subscribed, 1000).
-get_subscribed_and_online(FsmRef) ->
-    gen_fsm:sync_send_all_state_event(
-      FsmRef, get_subscribed_and_online, 1000).
-
 
 %%----------------------------------------------------------------------
 %% Func: StateName/2
@@ -1023,29 +1015,8 @@ handle_sync_event({get_presence}, _From, StateName, StateData) ->
     fsm_reply(Reply, StateName, StateData);
 
 handle_sync_event(get_subscribed, _From, StateName, StateData) ->
-    Subscribed = StateData#state.pres_f,
+    Subscribed = ?SETS:to_list(StateData#state.pres_f),
-    Online = StateData#state.pres_available,
+    {reply, Subscribed, StateName, StateData};
-    Pred = fun(User, _Caps) ->
-		   ?SETS:is_element(jlib:jid_remove_resource(User),
-				    Subscribed) orelse
-		       ?SETS:is_element(User, Subscribed)
-	   end,
-    SubscribedAndOnline = ?DICT:filter(Pred, Online),
-    SubscribedWithCaps  = ?SETS:fold(fun(User, Acc) ->
-	    [{User, undefined}|Acc]
-	end, ?DICT:to_list(SubscribedAndOnline), Subscribed),
-    {reply, SubscribedWithCaps, StateName, StateData};
-
-handle_sync_event(get_subscribed_and_online, _From, StateName, StateData) ->
-    Subscribed = StateData#state.pres_f,
-    Online = StateData#state.pres_available,
-    Pred = fun(User, _Caps) ->
-		   ?SETS:is_element(jlib:jid_remove_resource(User),
-				    Subscribed) orelse
-		       ?SETS:is_element(User, Subscribed)
-	   end,
-    SubscribedAndOnline = ?DICT:filter(Pred, Online),
-    {reply, ?DICT:to_list(SubscribedAndOnline), StateName, StateData};
 
 handle_sync_event(_Event, _From, StateName, StateData) ->
     Reply = ok,
@@ -1138,41 +1109,39 @@ handle_info({route, From, To, Packet}, StateName, StateData) ->
 				LFrom = jlib:jid_tolower(From),
 				LBFrom = jlib:jid_remove_resource(LFrom),
 				%% Note contact availability
-				Caps = mod_caps:read_caps(Els),
+				case xml:get_attr_s("type", Attrs) of
-				mod_caps:note_caps(StateData#state.server, From, Caps),
-				NewAvailable = case xml:get_attr_s("type", Attrs) of
 				    "unavailable" -> 
-						       ?DICT:erase(LFrom, StateData#state.pres_available);
+					mod_caps:clear_caps(From);
 				    _ -> 
-						       ?DICT:store(LFrom, Caps, StateData#state.pres_available)
+					Caps = mod_caps:read_caps(Els),
+					mod_caps:note_caps(StateData#state.server, From, Caps)
 				end,
-				NewStateData = StateData#state{pres_available = NewAvailable},
 				case ?SETS:is_element(
-					LFrom, NewStateData#state.pres_a) orelse
+					LFrom, StateData#state.pres_a) orelse
 				    ?SETS:is_element(
-				       LBFrom, NewStateData#state.pres_a) of
+				       LBFrom, StateData#state.pres_a) of
 				    true ->
-					{true, Attrs, NewStateData};
+					{true, Attrs, StateData};
 				    false ->
 					case ?SETS:is_element(
-						LFrom, NewStateData#state.pres_f) of
+						LFrom, StateData#state.pres_f) of
 					    true ->
 						A = ?SETS:add_element(
 						       LFrom,
-						       NewStateData#state.pres_a),
+						       StateData#state.pres_a),
 						{true, Attrs,
-						 NewStateData#state{pres_a = A}};
+						 StateData#state{pres_a = A}};
 					    false ->
 						case ?SETS:is_element(
-							LBFrom, NewStateData#state.pres_f) of
+							LBFrom, StateData#state.pres_f) of
 						    true ->
 							A = ?SETS:add_element(
 							       LBFrom,
-							       NewStateData#state.pres_a),
+							       StateData#state.pres_a),
 							{true, Attrs,
-							 NewStateData#state{pres_a = A}};
+							 StateData#state{pres_a = A}};
 						    false ->
-							{true, Attrs, NewStateData}
+							{true, Attrs, StateData}
 						end
 					end
 				end;

src/mod_caps.erl

@@ -31,7 +31,9 @@
 -behaviour(gen_mod).
 
 -export([read_caps/1,
+	 get_caps/1,
 	 note_caps/3,
+	 clear_caps/1,
 	 get_features/2,
 	 handle_disco_response/3]).
 
@@ -57,6 +59,7 @@
 
 -record(caps, {node, version, exts}).
 -record(caps_features, {node_pair, features}).
+-record(user_caps, {jid, caps}).
 -record(state, {host,
 		disco_requests = ?DICT:new(),
 		feature_queries = []}).
@@ -89,12 +92,26 @@ read_caps([_ | Tail], Result) ->
 read_caps([], Result) ->
     Result.
 
+%% get_caps reads user caps from database
+get_caps(JID) ->
+    case catch mnesia:dirty_read({user_caps, list_to_binary(jlib:jid_to_string(JID))}) of
+	[#user_caps{caps=Caps}] -> 
+	    Caps;
+	_ -> 
+	    nothing
+    end.
+
+%% clear_caps removes user caps from database
+clear_caps(JID) ->
+    catch mnesia:dirty_delete({user_caps, list_to_binary(jlib:jid_to_string(JID))}).
+
 %% note_caps should be called to make the module request disco
 %% information.  Host is the host that asks, From is the full JID that
 %% sent the caps packet, and Caps is what read_caps returned.
 note_caps(Host, From, Caps) ->
     case Caps of
-	nothing -> ok;
+	nothing -> 
+	    ok;
 	_ ->
 	    Proc = gen_mod:get_module_proc(Host, ?PROCNAME),
 	    gen_server:cast(Proc, {note_caps, From, Caps})
@@ -138,7 +155,9 @@ init([Host, _Opts]) ->
     mnesia:create_table(caps_features,
 			[{ram_copies, [node()]},
 			 {attributes, record_info(fields, caps_features)}]),
-    mnesia:add_table_copy(caps_features, node(), ram_copies),
+    mnesia:create_table(user_caps,
+			[{disc_copies, [node()]},
+			 {attributes, record_info(fields, user_caps)}]),
     {ok, #state{host = Host}}.
 
 maybe_get_features(#caps{node = Node, version = Version, exts = Exts}) ->
@@ -186,10 +205,12 @@ handle_call(stop, _From, State) ->
     {stop, normal, ok, State}.
 
 handle_cast({note_caps, From, 
-	     #caps{node = Node, version = Version, exts = Exts}}, 
+	     #caps{node = Node, version = Version, exts = Exts} = Caps}, 
 	    #state{host = Host, disco_requests = Requests} = State) ->
     %% XXX: this leads to race conditions where ejabberd will send
     %% lots of caps disco requests.
+    mnesia:dirty_write(#user_caps{jid = list_to_binary(jlib:jid_to_string(From)),
+				  caps = Caps}),
     SubNodes = [Version | Exts],
     %% Now, find which of these are not already in the database.
     Fun = fun() ->
@@ -204,10 +225,8 @@ handle_cast({note_caps, From,
 	  end,
     case mnesia:transaction(Fun) of
 	{atomic, Missing} ->
-	    %% For each unknown caps "subnode", we send a disco
+	    %% For each unknown caps "subnode", we send a disco request.
-	    %% request.
+	    NewRequests = lists:foldl(
-	    NewRequests =
-		lists:foldl(
 		fun(SubNode, Dict) ->
 			  ID = randoms:get_string(),
 			  Stanza =

src/mod_pubsub/mod_pubsub.erl

@@ -473,17 +473,14 @@ handle_cast({presence, JID, Pid}, State) ->
     end, State#state.plugins),
     %% and send to From last PEP events published by its contacts
     case catch ejabberd_c2s:get_subscribed(Pid) of
-	ContactsWithCaps when is_list(ContactsWithCaps) ->
+	Contacts when is_list(Contacts) ->
-	    Caps = proplists:get_value(LJID, ContactsWithCaps),
-	    ContactsUsers = lists:usort(lists:map(
-				fun({{User, Server, _}, _}) -> {User, Server} end, ContactsWithCaps)),
 	    lists:foreach(
-		fun({User, Server}) ->
+		fun({User, Server, _}) ->
-		    PepKey = {User, Server, ""},
+		    Owner = {User, Server, ""},
 		    lists:foreach(fun(#pubsub_node{nodeid = {_, Node}, options = Options}) ->
 			case get_option(Options, send_last_published_item) of
 			    on_sub_and_presence ->
-				case is_caps_notify(ServerHost, Node, Caps) of
+				case is_caps_notify(ServerHost, Node, LJID) of
 				    true ->
 					Subscribed = case get_option(Options, access_model) of
 					    open -> true;
@@ -495,8 +492,7 @@ handle_cast({presence, JID, Pid}, State) ->
 						element(2, get_roster_info(User, Server, LJID, Grps))
 					end,
 					if Subscribed ->
-					    ?DEBUG("send ~s's ~s event to ~s",[jlib:jid_to_string(PepKey),Node,jlib:jid_to_string(LJID)]),
+					    send_last_item(Owner, Node, LJID);
-					    send_last_item(PepKey, Node, LJID);
 					true ->
 					    ok
 					end;
@@ -506,8 +502,8 @@ handle_cast({presence, JID, Pid}, State) ->
 			    _ ->
 				ok
 			end
-		    end, tree_action(Host, get_nodes, [PepKey]))
+		    end, tree_action(Host, get_nodes, [Owner]))
-		end, ContactsUsers);
+		end, Contacts);
 	_ ->
 	    ok
     end,
@@ -2321,25 +2317,7 @@ broadcast_config_notification(Host, Node, Lang) ->
 %% broadcast Stanza to all contacts of the user that are advertising
 %% interest in this kind of Node.
 broadcast_by_caps({LUser, LServer, LResource}, Node, _Type, Stanza) ->
-    ?DEBUG("looking for pid of ~p@~p/~p", [LUser, LServer, LResource]),
+    SenderResource = user_resource(LUser, LServer, LResource),
-    %% We need to know the resource, so we can ask for presence data.
-    SenderResource = case LResource of
-			 "" ->
-			     %% If we don't know the resource, just pick one.
-			     case ejabberd_sm:get_user_resources(LUser, LServer) of
-				 [R|_] ->
-				     R;
-				 [] ->
-				     ""
-			     end;
-			 _ ->
-			     LResource
-		     end,
-    case SenderResource of
-    "" ->
-	?DEBUG("~p@~p is offline; can't deliver ~p to contacts", [LUser, LServer, Stanza]),
-	ok;
-    _ ->
     case ejabberd_sm:get_session_pid(LUser, LServer, SenderResource) of
 	C2SPid when is_pid(C2SPid) ->
 	    %% set the from address on the notification to the bare JID of the account owner
@@ -2347,19 +2325,22 @@ broadcast_by_caps({LUser, LServer, LResource}, Node, _Type, Stanza) ->
 	    %% See XEP-0163 1.1 section 4.3.1
 	    Sender = jlib:make_jid(LUser, LServer, ""),
 	    %%ReplyTo = jlib:make_jid(LUser, LServer, SenderResource),  % This has to be used
-		case catch ejabberd_c2s:get_subscribed_and_online(C2SPid) of
+	    case catch ejabberd_c2s:get_subscribed(C2SPid) of
-		    ContactsWithCaps when is_list(ContactsWithCaps) ->
+		Contacts when is_list(Contacts) ->
-			?DEBUG("found contacts with caps: ~p", [ContactsWithCaps]),
+		    Online = lists:foldl(fun({U, S, R}, Acc) ->
-			lists:foreach(
+				case user_resource(U, S, R) of
-			fun({JID, Caps}) ->
+				    [] -> Acc;
-				case is_caps_notify(LServer, Node, Caps) of
+				    OR -> [{U, S, OR}|Acc]
+				end
+			    end, [], Contacts),
+		    lists:foreach(fun(LJID) ->
+			case is_caps_notify(LServer, Node, LJID) of
 			    true ->
-					To = jlib:make_jid(JID),
+				ejabberd_router ! {route, Sender, jlib:make_jid(LJID), Stanza};
-					ejabberd_router ! {route, Sender, To, Stanza};
 			    false ->
 				ok
 			end
-			end, ContactsWithCaps);
+		    end, Online);
 		_ ->
 		    ok
 	    end,
@@ -2367,12 +2348,21 @@ broadcast_by_caps({LUser, LServer, LResource}, Node, _Type, Stanza) ->
 	_ ->
 	    ?DEBUG("~p@~p has no session; can't deliver ~p to contacts", [LUser, LServer, Stanza]),
 	    ok
-	end
     end;
 broadcast_by_caps(_, _, _, _) ->
     ok.
 
-is_caps_notify(Host, Node, Caps) ->
+user_resource(LUser, LServer, []) ->
+    %% If we don't know the resource, just pick first if any
+    case ejabberd_sm:get_user_resources(LUser, LServer) of
+	[R|_] -> R;
+	[] -> []
+    end;
+user_resource(_, _, LResource) ->
+    LResource.
+
+is_caps_notify(Host, Node, LJID) ->
+    Caps = mod_caps:get_caps(LJID),
     case catch mod_caps:get_features(Host, Caps) of
 	Features when is_list(Features) -> lists:member(Node ++ "+notify", Features);
 	_ -> false