Reworked the permissions cache to improve memory usage and performance

src/Discord.Net/Models/Channel.cs

@@ -8,6 +8,19 @@ namespace Discord
 {
 	public sealed class Channel : CachedObject<long>
 	{
+		private struct ChannelMember
+		{
+			public readonly User User;
+			public readonly ChannelPermissions Permissions;
+
+			public ChannelMember(User user)
+			{
+				User = user;
+				Permissions = new ChannelPermissions();
+				Permissions.Lock();
+			}
+		}
+
 		public sealed class PermissionOverwrite
 		{
 			public PermissionTarget TargetType { get; }
@@ -22,7 +35,7 @@ namespace Discord
 				Permissions.Lock();
 			}
 		}
-				
+
 		/// <summary> Returns the name of this channel. </summary>
 		public string Name { get; private set; }
 		/// <summary> Returns the topic associated with this channel. </summary>
@@ -50,13 +63,15 @@ namespace Discord
 		{
 			get
 			{
-				if (_areMembersStale)
+				if (Type == ChannelType.Text)
-					UpdateMembersCache();
+					return _members.Values.Where(x => x.Permissions.ReadMessages == true).Select(x => x.User);
-				return _members.Select(x => x.Value);
+				else if (Type == ChannelType.Voice)
+					return Server.Members.Where(x => x.VoiceChannel == this);
+				else
+					return Enumerable.Empty<User>();
             }
 		}
-		private Dictionary<long, User> _members;
+		private ConcurrentDictionary<long, ChannelMember> _members;
-		private bool _areMembersStale;
 
 		/// <summary> Returns a collection of all messages the client has seen posted in this channel. This collection does not guarantee any ordering. </summary>
 		[JsonIgnore]
@@ -89,7 +104,13 @@ namespace Discord
 						x.GlobalUser.PrivateChannel = null;
                 });
 			_permissionOverwrites = _initialPermissionsOverwrites;
-			_areMembersStale = true;
+			_members = new ConcurrentDictionary<long, ChannelMember>();
+
+			if (recipientId != null)
+			{
+				AddMember(client.PrivateUser);
+				AddMember(Recipient);
+			}
 
 			//Local Cache
 			if (client.Config.MessageCacheLength > 0)
@@ -135,7 +156,7 @@ namespace Discord
 				_permissionOverwrites = model.PermissionOverwrites
 					.Select(x => new PermissionOverwrite(PermissionTarget.FromString(x.Type), x.Id, x.Allow, x.Deny))
 					.ToArray();
-				InvalidatePermissionsCache();
+				UpdatePermissions();
             }
 		}
 
@@ -156,52 +177,82 @@ namespace Discord
 			}
 		}
 		internal void RemoveMessage(Message message) => _messages.TryRemove(message.Id, out message);
-
+		
-		internal void InvalidateMembersCache()
+		internal void AddMember(User user)
 		{
-			_areMembersStale = true;
+			var member = new ChannelMember(user);
+            if (_members.TryAdd(user.Id, member))
+				UpdatePermissions(user, member.Permissions);
+        }
+		internal void RemoveMember(User user)
+		{
+			ChannelMember ignored;
+			_members.TryRemove(user.Id, out ignored);
 		}
-		private void UpdateMembersCache()
+
+		internal ChannelPermissions GetPermissions(User user)
 		{
-			if (IsPrivate)
+			ChannelMember member;
+			if (_members.TryGetValue(user.Id, out member))
+				return member.Permissions;
+			else
+				return null;
+		}
+		internal void UpdatePermissions()
+		{
+			foreach (var pair in _members)
 			{
-				_members = new Dictionary<long, User>()
+				ChannelMember member = pair.Value;
+				UpdatePermissions(member.User, member.Permissions);
+			}
+		}
+		internal void UpdatePermissions(User user)
+		{
+			ChannelMember member;
+			if (_members.TryGetValue(user.Id, out member))
+				UpdatePermissions(member.User, member.Permissions);
+        }
+        private void UpdatePermissions(User user, ChannelPermissions permissions)
+		{
+			uint newPermissions = 0;
+			var server = Server;
+
+			//Load the mask of all permissions supported by this channel type
+			var mask = ChannelPermissions.All(this).RawValue;
+
+			if (server != null)
+			{
+				//Start with this user's server permissions
+				newPermissions = server.GetPermissions(user).RawValue;
+
+				if (IsPrivate || server.Owner == user)
+					newPermissions = mask; //Owners always have all permissions
+				else
 				{
-					{ _client.CurrentUserId, _client.PrivateUser },
+					var channelOverwrites = PermissionOverwrites;
-					{ _recipient.Id.Value, _recipient.Value }
-				};
-			}
-			else if (Type == ChannelType.Text)
-			{
-				_members = Server.Members
-					.Where(x => x.GetPermissions(this)?.ReadMessages ?? false)
-					.ToDictionary(x => x.Id, x => x);
-			}
-			else if (Type == ChannelType.Voice)
-			{
-				_members = Server.Members
-					.Where(x => x.VoiceChannel == this)
-					.ToDictionary(x => x.Id, x => x);
-			}
-			_areMembersStale = false;
-		}
 
-		internal void InvalidatePermissionsCache()
+					var roles = user.Roles;
-		{
+					foreach (var denyRole in channelOverwrites.Where(x => x.TargetType == PermissionTarget.Role && x.Permissions.Deny.RawValue != 0 && roles.Any(y => y.Id == x.TargetId)))
-			UpdateMembersCache();
+						newPermissions &= ~denyRole.Permissions.Deny.RawValue;
-			foreach (var member in _members)
+					foreach (var allowRole in channelOverwrites.Where(x => x.TargetType == PermissionTarget.Role && x.Permissions.Allow.RawValue != 0 && roles.Any(y => y.Id == x.TargetId)))
-				member.Value.UpdateChannelPermissions(this);
+						newPermissions |= allowRole.Permissions.Allow.RawValue;
-		}
+					foreach (var denyUser in channelOverwrites.Where(x => x.TargetType == PermissionTarget.User && x.TargetId == Id && x.Permissions.Deny.RawValue != 0))
-		/*internal void InvalidatePermissionsCache(Role role)
+						newPermissions &= ~denyUser.Permissions.Deny.RawValue;
-		{
+					foreach (var allowUser in channelOverwrites.Where(x => x.TargetType == PermissionTarget.User && x.TargetId == Id && x.Permissions.Allow.RawValue != 0))
-			_areMembersStale = true;
+						newPermissions |= allowUser.Permissions.Allow.RawValue;
-			foreach (var member in role.Members)
+
-				member.UpdateChannelPermissions(this);
+					if (BitHelper.GetBit(newPermissions, (int)PermissionsBits.ManageRolesOrPermissions))
-		}*/
+						newPermissions = mask; //ManageRolesOrPermissions gives all permisions
-		internal void InvalidatePermissionsCache(User user)
+					else if (Type == ChannelType.Text && !BitHelper.GetBit(newPermissions, (int)PermissionsBits.ReadMessages))
-		{
+						newPermissions = 0; //No read permission on a text channel removes all other permissions
-			_areMembersStale = true;
+					else
-			user.UpdateChannelPermissions(this);
+						newPermissions &= mask; //Ensure we didnt get any permissions this channel doesnt support (from serverPerms, for example)
+				}
+			}
+			else
+				newPermissions = mask; //Private messages always have all permissions
+
+			permissions.SetRawValueInternal(newPermissions);
 		}
 
 		public override bool Equals(object obj) => obj is Channel && (obj as Channel).Id == Id;

src/Discord.Net/Models/Role.cs

@@ -67,7 +67,7 @@ namespace Discord
 				Permissions.SetRawValueInternal(model.Permissions.Value);
 
 			foreach (var member in Members)
-				member.UpdateServerPermissions();
+				Server.UpdatePermissions(member);
 		}
 
 		public override bool Equals(object obj) => obj is Role && (obj as Role).Id == Id;

src/Discord.Net/Models/Server.cs

@@ -8,7 +8,20 @@ using System.Linq;
 namespace Discord
 {
 	public sealed class Server : CachedObject<long>
-	{		
+	{
+		private struct ServerMember
+		{
+			public readonly User User;
+			public readonly ServerPermissions Permissions;
+
+			public ServerMember(User user)
+			{
+				User = user;
+				Permissions = new ServerPermissions();
+				Permissions.Lock();
+			}
+		}
+		
 		/// <summary> Returns the name of this channel. </summary>
 		public string Name { get; private set; }
 		/// <summary> Returns the current logged-in user's data for this server. </summary>
@@ -20,8 +33,6 @@ namespace Discord
 		public DateTime JoinedAt { get; private set; }
 		/// <summary> Returns the region for this server (see Regions). </summary>
 		public string Region { get; private set; }
-		/*/// <summary> Returns the endpoint for this server's voice server. </summary>
-		internal string VoiceServer { get; set; }*/
 
 		/// <summary> Returns true if the current user created this server. </summary>
 		public bool IsOwner => _client.CurrentUserId == _ownerId;
@@ -47,18 +58,18 @@ namespace Discord
 		/// <summary> Returns a collection of all channels within this server. </summary>
 		[JsonIgnore]
 		public IEnumerable<Channel> Channels => _channels.Select(x => x.Value);
-		/// <summary> Returns a collection of all channels within this server. </summary>
+		/// <summary> Returns a collection of all text channels within this server. </summary>
 		[JsonIgnore]
 		public IEnumerable<Channel> TextChannels => _channels.Select(x => x.Value).Where(x => x.Type == ChannelType.Text);
-		/// <summary> Returns a collection of all channels within this server. </summary>
+		/// <summary> Returns a collection of all voice channels within this server. </summary>
 		[JsonIgnore]
 		public IEnumerable<Channel> VoiceChannels => _channels.Select(x => x.Value).Where(x => x.Type == ChannelType.Voice);
 		private ConcurrentDictionary<long, Channel> _channels;
 
 		/// <summary> Returns a collection of all users within this server with their server-specific data. </summary>
 		[JsonIgnore]
-		public IEnumerable<User> Members => _members.Select(x => x.Value);
+		public IEnumerable<User> Members => _members.Select(x => x.Value.User);
-		private ConcurrentDictionary<long, User> _members;
+		private ConcurrentDictionary<long, ServerMember> _members;
 
 		/// <summary> Return the the role representing all users in a server. </summary>
 		[JsonIgnore]
@@ -75,8 +86,8 @@ namespace Discord
 
 			//Global Cache
 			_channels = new ConcurrentDictionary<long, Channel>();
-			_members = new ConcurrentDictionary<long, User>();
 			_roles = new ConcurrentDictionary<long, Role>();
+			_members = new ConcurrentDictionary<long, ServerMember>();
 
 			//Local Cache
 			_bans = new ConcurrentDictionary<long, bool>();
@@ -194,43 +205,34 @@ namespace Discord
 			{
 				if (channel.Id == Id)
 					DefaultChannel = channel;
-				foreach (var member in Members)
-					member.AddChannel(channel);
 			}
 		}
 		internal void RemoveChannel(Channel channel)
 		{
-			foreach (var member in Members)
-				member.RemoveChannel(channel);
 			_channels.TryRemove(channel.Id, out channel);
 		}
 
 		internal void AddMember(User user)
 		{
-			if (_members.TryAdd(user.Id, user))
+			if (_members.TryAdd(user.Id, new ServerMember(user)))
 			{
 				if (user.Id == _ownerId)
 					Owner = user;
 
 				foreach (var channel in TextChannels)
-				{
+					channel.AddMember(user);
-					user.AddChannel(channel);
-					channel.InvalidatePermissionsCache(user);
-				}
 			}
         }
 		internal void RemoveMember(User user)
 		{
-			if (_members.TryRemove(user.Id, out user))
+			ServerMember ignored;
+			if (_members.TryRemove(user.Id, out ignored))
 			{
 				if (user.Id == _ownerId)
 					Owner = null;
 
 				foreach (var channel in Channels)
-				{
+					channel.RemoveMember(user);
-					user.RemoveChannel(channel);
-					channel.InvalidatePermissionsCache(user);
-				}
 			}
 		}
 		internal void HasMember(User user) => _members.ContainsKey(user.Id);
@@ -252,6 +254,45 @@ namespace Discord
 			}
 		}
 
+		internal ServerPermissions GetPermissions(User user)
+		{
+			ServerMember member;
+			if (_members.TryGetValue(user.Id, out member))
+				return member.Permissions;
+			else
+				return null;
+		}
+		internal void UpdatePermissions(User user)
+		{
+			ServerMember member;
+			if (_members.TryGetValue(user.Id, out member))
+				UpdatePermissions(member.User, member.Permissions);
+		}
+        private void UpdatePermissions(User user, ServerPermissions permissions)
+		{
+			uint oldPermissions = permissions.RawValue;
+			uint newPermissions = 0;
+
+			if (Owner == user)
+				newPermissions = ServerPermissions.All.RawValue;
+			else
+			{
+				var roles = Roles;
+				foreach (var serverRole in roles)
+					newPermissions |= serverRole.Permissions.RawValue;
+			}
+
+			if (BitHelper.GetBit(newPermissions, (int)PermissionsBits.ManageRolesOrPermissions))
+				newPermissions = ServerPermissions.All.RawValue;
+
+			if (newPermissions != oldPermissions)
+			{
+				permissions.SetRawValueInternal(newPermissions);
+				foreach (var channel in _channels)
+					channel.Value.UpdatePermissions(user);
+			}
+		}
+
 		public override bool Equals(object obj) => obj is Server && (obj as Server).Id == Id;
 		public override int GetHashCode() => unchecked(Id.GetHashCode() + 5175);
 		public override string ToString() => Name ?? IdConvert.ToString(Id);

src/Discord.Net/Models/User.cs

@@ -7,19 +7,6 @@ using System.Linq;
 
 namespace Discord
 {
-	public struct ChannelPermissionsPair
-	{
-		public Channel Channel;
-		public ChannelPermissions Permissions;
-
-		public ChannelPermissionsPair(Channel channel)
-		{
-			Channel = channel;
-			Permissions = new ChannelPermissions();
-			Permissions.Lock();
-        }
-	}
-
 	public class User : CachedObject<long>
 	{
 		internal struct CompositeKey : IEquatable<CompositeKey>
@@ -38,9 +25,6 @@ namespace Discord
 		}
 
 		internal static string GetAvatarUrl(long userId, string avatarId) => avatarId != null ? Endpoints.UserAvatar(userId, avatarId) : null;
-		
-		private ConcurrentDictionary<long, ChannelPermissionsPair> _permissions;
-		private ServerPermissions _serverPermissions;
 
 		/// <summary> Returns a unique identifier combining this user's id with its server's. </summary>
 		internal CompositeKey UniqueId => new CompositeKey(_server.Id ?? 0, Id);
@@ -117,10 +101,8 @@ namespace Discord
 			{
 				if (_server.Id != null)
 				{
-					return _permissions
+					return Server.Channels
-						.Where(x => x.Value.Permissions.ReadMessages)
+						.Where(x => x.GetPermissions(this).ReadMessages);
-						.Select(x => x.Value.Channel)
-						.Where(x => x != null);
 				}
 				else
 				{
@@ -158,12 +140,6 @@ namespace Discord
 			_roles = new Dictionary<long, Role>();
 
 			Status = UserStatus.Offline;
-			//_channels = new ConcurrentDictionary<string, Channel>();
-			if (serverId != null)
-			{
-				_permissions = new ConcurrentDictionary<long, ChannelPermissionsPair>();
-				_serverPermissions = new ServerPermissions();
-			}
 
 			if (serverId == null)
 				UpdateRoles(null);
@@ -197,8 +173,6 @@ namespace Discord
 				JoinedAt = model.JoinedAt.Value;
 			if (model.Roles != null)
 				UpdateRoles(model.Roles.Select(x => _client.Roles[x]));
-
-			UpdateServerPermissions();
         }
 		internal void Update(ExtendedMemberInfo model)
 		{
@@ -242,20 +216,9 @@ namespace Discord
 				IsSelfMuted = model.IsSelfMuted.Value;
 			if (model.IsServerSuppressed != null)
 				IsServerSuppressed = model.IsServerSuppressed.Value;
-
+			
-			if (_voiceChannel.Id != model.ChannelId)
+			_voiceChannel.Id = model.ChannelId; //Allows null
-			{
+		}
-				var oldChannel = _voiceChannel.Value;
-				if (oldChannel != null)
-					oldChannel.InvalidateMembersCache();
-
-				_voiceChannel.Id = model.ChannelId; //Can be null
-
-				var newChannel = _voiceChannel.Value;
-				if (newChannel != null)
-					newChannel.InvalidateMembersCache();
-			}
-        }
 		private void UpdateRoles(IEnumerable<Role> roles)
 		{
 			Dictionary<long, Role> newRoles = new Dictionary<long, Role>();
@@ -271,6 +234,9 @@ namespace Discord
 				newRoles.Add(everyone.Id, everyone);
 			}
 			_roles = newRoles;
+
+			if (!IsPrivate)
+				Server.UpdatePermissions(this);
 		}
 
 		internal void UpdateActivity(DateTime? activity = null)
@@ -278,111 +244,13 @@ namespace Discord
 			if (LastActivityAt == null || activity > LastActivityAt.Value)
 				LastActivityAt = activity ?? DateTime.UtcNow;
 		}
-
+		
-		internal void UpdateServerPermissions()
+		public ServerPermissions ServerPermissions => Server.GetPermissions(this);
-		{
-			var server = Server;
-			if (server == null) return;
-
-			uint newPermissions = 0x0;
-			uint oldPermissions = _serverPermissions.RawValue;
-
-			if (server.Owner == this)
-				newPermissions = ServerPermissions.All.RawValue;
-			else
-			{
-				var roles = Roles;
-				foreach (var serverRole in roles)
-					newPermissions |= serverRole.Permissions.RawValue;
-			}
-
-			if (BitHelper.GetBit(newPermissions, (int)PermissionsBits.ManageRolesOrPermissions))
-				newPermissions = ServerPermissions.All.RawValue;
-
-			if (newPermissions != oldPermissions)
-			{
-				_serverPermissions.SetRawValueInternal(newPermissions);
-				foreach (var permission in _permissions)
-					UpdateChannelPermissions(permission.Value.Channel);
-			}
-		}
-		internal void UpdateChannelPermissions(Channel channel)
-		{
-			var server = Server;
-			if (server == null) return;
-			if (channel.Server != server) throw new InvalidOperationException();
-
-			ChannelPermissionsPair chanPerms;
-			if (!_permissions.TryGetValue(channel.Id, out chanPerms)) return;
-			uint newPermissions = _serverPermissions.RawValue;
-			uint oldPermissions = chanPerms.Permissions.RawValue;
-			
-			if (server.Owner == this)
-				newPermissions = ChannelPermissions.All(channel).RawValue;
-			else
-			{
-				var channelOverwrites = channel.PermissionOverwrites;
-
-				//var roles = Roles.OrderBy(x => x.Id);
-				var roles = Roles;
-				foreach (var denyRole in channelOverwrites.Where(x => x.TargetType == PermissionTarget.Role && x.Permissions.Deny.RawValue != 0 && roles.Any(y => y.Id == x.TargetId)))
-					newPermissions &= ~denyRole.Permissions.Deny.RawValue;
-				foreach (var allowRole in channelOverwrites.Where(x => x.TargetType == PermissionTarget.Role && x.Permissions.Allow.RawValue != 0 && roles.Any(y => y.Id == x.TargetId)))
-					newPermissions |= allowRole.Permissions.Allow.RawValue;
-				foreach (var denyUser in channelOverwrites.Where(x => x.TargetType == PermissionTarget.User && x.TargetId == Id && x.Permissions.Deny.RawValue != 0))
-					newPermissions &= ~denyUser.Permissions.Deny.RawValue;
-				foreach (var allowUser in channelOverwrites.Where(x => x.TargetType == PermissionTarget.User && x.TargetId == Id && x.Permissions.Allow.RawValue != 0))
-					newPermissions |= allowUser.Permissions.Allow.RawValue;
-			}
-
-			var mask = ChannelPermissions.All(channel).RawValue;
-			if (BitHelper.GetBit(newPermissions, (int)PermissionsBits.ManageRolesOrPermissions))
-				newPermissions = mask;
-			else if (!BitHelper.GetBit(newPermissions, (int)PermissionsBits.ReadMessages))
-				newPermissions = ChannelPermissions.None.RawValue;
-			else
-				newPermissions &= mask;
-
-			if (newPermissions != oldPermissions)
-			{
-				chanPerms.Permissions.SetRawValueInternal(newPermissions);
-				channel.InvalidateMembersCache();
-			}
-
-			chanPerms.Permissions.SetRawValueInternal(newPermissions);
-		}
-
-		public ServerPermissions GetServerPermissions() => _serverPermissions;
 		public ChannelPermissions GetPermissions(Channel channel)
 		{
 			if (channel == null) throw new ArgumentNullException(nameof(channel));
 
-			//Return static permissions if this is a private chat
+			return channel.GetPermissions(this);
-			if (_server.Id == null)
-				return ChannelPermissions.PrivateOnly;
-
-			ChannelPermissionsPair chanPerms;
-			if (_permissions.TryGetValue(channel.Id, out chanPerms))
-				return chanPerms.Permissions;
-			return null;
-		}
-
-		internal void AddChannel(Channel channel)
-		{
-			if (_server.Id != null)
-			{
-				_permissions.TryAdd(channel.Id, new ChannelPermissionsPair(channel));
-				UpdateChannelPermissions(channel);
-			}
-		}
-		internal void RemoveChannel(Channel channel)
-		{
-			if (_server.Id != null)
-			{
-				ChannelPermissionsPair ignored;
-				//_channels.TryRemove(channel.Id, out channel);
-				_permissions.TryRemove(channel.Id, out ignored);
-			}
 		}
 
 		public bool HasRole(Role role)