Reduce User* -> Membership* lookups on part and kick

4 files changed, 77 insertions, 39 deletions

diff --git a/include/channels.h b/include/channels.h
index 44198724c..d1531609a 100644
--- a/include/channels.h
+++ b/include/channels.h
@@ -52,12 +52,32 @@ class CoreExport Channel : public Extensible, public InviteBase
 	 */
 	CustomModeList custom_mode_params;
 
+	/** Remove the given membership from the channel's internal map of
+	 * memberships and destroy the Membership object.
+	 * This function does not remove the channel from User::chanlist.
+	 * Since the parameter is an iterator to the target, the complexity
+	 * of this function is constant.
+	 * @param membiter The UserMembIter to remove, must be valid
+	 */
+	void DelUser(const UserMembIter& membiter);
+
  public:
 	/** Creates a channel record and initialises it with default values
 	 * @throw Nothing at present.
 	 */
 	Channel(const std::string &name, time_t ts);
 
+	/** Checks whether the channel should be destroyed, and if yes, begins
+	 * the teardown procedure.
+	 *
+	 * If there are users on the channel or a module vetoes the deletion
+	 * (OnPreChannelDelete hook) then nothing else happens.
+	 * Otherwise, first the OnChannelDelete event is fired, then the channel is
+	 * removed from the channel list. All pending invites are destroyed and
+	 * finally the channel is added to the cull list.
+	 */
+	void CheckDestroy();
+
 	/** The channel's name.
 	 */
 	std::string name;
@@ -173,8 +193,9 @@ class CoreExport Channel : public Extensible, public InviteBase
 	 * @param src The source of the kick
 	 * @param user The user being kicked (must be on this channel)
 	 * @param reason The reason for the kick
+	 * @param srcmemb The membership of the user who does the kick, can be NULL
 	 */
-	void KickUser(User *src, User *user, const std::string& reason);
+	void KickUser(User* src, User* user, const std::string& reason, Membership* srcmemb = NULL);
 
 	/** Part a user from this channel with the given reason.
 	 * If the reason field is NULL, no reason will be sent.
diff --git a/src/channels.cpp b/src/channels.cpp
index 2807249c1..2b347a69e 100644
--- a/src/channels.cpp
+++ b/src/channels.cpp
@@ -138,32 +138,42 @@ Membership* Channel::AddUser(User* user)
 
 void Channel::DelUser(User* user)
 {
-	UserMembIter a = userlist.find(user);
+	UserMembIter it = userlist.find(user);
+	if (it != userlist.end())
+		DelUser(it);
+}
+
+void Channel::CheckDestroy()
+{
+	if (!userlist.empty())
+		return;
+
+	ModResult res;
+	FIRST_MOD_RESULT(OnChannelPreDelete, res, (this));
+	if (res == MOD_RES_DENY)
+		return;
 
-	if (a != userlist.end())
+	chan_hash::iterator iter = ServerInstance->chanlist->find(this->name);
+	/* kill the record */
+	if (iter != ServerInstance->chanlist->end())
 	{
-		a->second->cull();
-		delete a->second;
-		userlist.erase(a);
+		FOREACH_MOD(I_OnChannelDelete, OnChannelDelete(this));
+		ServerInstance->chanlist->erase(iter);
 	}
 
-	if (userlist.empty())
-	{
-		ModResult res;
-		FIRST_MOD_RESULT(OnChannelPreDelete, res, (this));
-		if (res == MOD_RES_DENY)
-			return;
-		chan_hash::iterator iter = ServerInstance->chanlist->find(this->name);
-		/* kill the record */
-		if (iter != ServerInstance->chanlist->end())
-		{
-			FOREACH_MOD(I_OnChannelDelete, OnChannelDelete(this));
-			ServerInstance->chanlist->erase(iter);
-		}
+	ClearInvites();
+	ServerInstance->GlobalCulls.AddItem(this);
+}
 
-		ClearInvites();
-		ServerInstance->GlobalCulls.AddItem(this);
-	}
+void Channel::DelUser(const UserMembIter& membiter)
+{
+	Membership* memb = membiter->second;
+	memb->cull();
+	delete memb;
+	userlist.erase(membiter);
+
+	// If this channel became empty then it should be removed
+	CheckDestroy();
 }
 
 Membership* Channel::GetUser(User* user)
@@ -481,38 +491,44 @@ ModResult Channel::GetExtBanStatus(User *user, char type)
 }
 
 /* Channel::PartUser
- * remove a channel from a users record, and return the number of users left.
- * Therefore, if this function returns 0 the caller should delete the Channel.
+ * Remove a channel from a users record, remove the reference to the Membership object
+ * from the channel and destroy it.
  */
 void Channel::PartUser(User *user, std::string &reason)
 {
-	Membership* memb = GetUser(user);
+	UserMembIter membiter = userlist.find(user);
 
-	if (memb)
+	if (membiter != userlist.end())
 	{
+		Membership* memb = membiter->second;
 		CUList except_list;
 		FOREACH_MOD(I_OnUserPart,OnUserPart(memb, reason, except_list));
 
 		WriteAllExcept(user, false, 0, except_list, "PART %s%s%s", this->name.c_str(), reason.empty() ? "" : " :", reason.c_str());
 
+		// Remove this channel from the user's chanlist
 		user->chans.erase(this);
-		this->RemoveAllPrefixes(user);
+		// Remove the Membership from this channel's userlist and destroy it
+		this->DelUser(membiter);
 	}
-
-	this->DelUser(user);
 }
 
-void Channel::KickUser(User* src, User* victim, const std::string& reason)
+void Channel::KickUser(User* src, User* victim, const std::string& reason, Membership* srcmemb)
 {
-	Membership* memb = GetUser(victim);
+	UserMembIter victimiter = userlist.find(victim);
+	Membership* memb = ((victimiter != userlist.end()) ? victimiter->second : NULL);
+
 	if (!memb)
 	{
 		src->WriteNumeric(ERR_USERNOTINCHANNEL, "%s %s %s :They are not on that channel",src->nick.c_str(), victim->nick.c_str(), this->name.c_str());
 		return;
 	}
 
+	// Do the following checks only if the KICK is done by a local user;
+	// each server enforces its own rules.
 	if (IS_LOCAL(src))
 	{
+		// Modules are allowed to explicitly allow or deny kicks done by local users
 		ModResult res;
 		FIRST_MOD_RESULT(OnUserPreKick, res, (src,memb,reason));
 		if (res == MOD_RES_DENY)
@@ -520,7 +536,9 @@ void Channel::KickUser(User* src, User* victim, const std::string& reason)
 
 		if (res == MOD_RES_PASSTHRU)
 		{
-			unsigned int them = this->GetPrefixValue(src);
+			if (!srcmemb)
+				srcmemb = GetUser(src);
+			unsigned int them = srcmemb ? srcmemb->getRank() : 0;
 			unsigned int req = HALFOP_VALUE;
 			for (std::string::size_type i = 0; i < memb->modes.length(); i++)
 			{
@@ -544,7 +562,7 @@ void Channel::KickUser(User* src, User* victim, const std::string& reason)
 	WriteAllExcept(src, false, 0, except_list, "KICK %s %s :%s", name.c_str(), victim->nick.c_str(), reason.c_str());
 
 	victim->chans.erase(this);
-	this->DelUser(victim);
+	this->DelUser(victimiter);
 }
 
 void Channel::WriteChannel(User* user, const char* text, ...)
diff --git a/src/commands/cmd_kick.cpp b/src/commands/cmd_kick.cpp
index ecbdbad1e..016a14b2e 100644
--- a/src/commands/cmd_kick.cpp
+++ b/src/commands/cmd_kick.cpp
@@ -62,9 +62,11 @@ CmdResult CommandKick::Handle (const std::vector<std::string>& parameters, User
 		return CMD_FAILURE;
 	}
 
+	Membership* srcmemb = NULL;
 	if (IS_LOCAL(user))
 	{
-		if (!c->HasUser(user))
+		srcmemb = c->GetUser(user);
+		if (!srcmemb)
 		{
 			user->WriteServ( "442 %s %s :You're not on that channel!", user->nick.c_str(), parameters[0].c_str());
 			return CMD_FAILURE;
@@ -86,7 +88,7 @@ CmdResult CommandKick::Handle (const std::vector<std::string>& parameters, User
 		reason.assign(user->nick, 0, ServerInstance->Config->Limits.MaxKick);
 	}
 
-	c->KickUser(user, u, reason);
+	c->KickUser(user, u, reason, srcmemb);
 
 	return CMD_SUCCESS;
 }
diff --git a/src/modules/m_permchannels.cpp b/src/modules/m_permchannels.cpp
index 8aa888485..41477ba35 100644
--- a/src/modules/m_permchannels.cpp
+++ b/src/modules/m_permchannels.cpp
@@ -143,10 +143,7 @@ class PermChannel : public ModeHandler
 			if (channel->IsModeSet('P'))
 			{
 				channel->SetMode(this,false);
-				if (channel->GetUserCounter() == 0)
-				{
-					channel->DelUser(ServerInstance->FakeClient);
-				}
+				channel->CheckDestroy();
 				return MODEACTION_ALLOW;
 			}
 		}