From 054b2f0af0e63790745e1401f8cf454e582cb280 Mon Sep 17 00:00:00 2001 From: RogueException Date: Tue, 24 Nov 2015 16:52:49 -0400 Subject: [PATCH] Reworked the permissions cache to improve memory usage and performance --- src/Discord.Net/Models/Channel.cs | 141 +++++++++++++++++++++++----------- src/Discord.Net/Models/Role.cs | 2 +- src/Discord.Net/Models/Server.cs | 85 +++++++++++++++------ src/Discord.Net/Models/User.cs | 154 +++----------------------------------- 4 files changed, 171 insertions(+), 211 deletions(-) diff --git a/src/Discord.Net/Models/Channel.cs b/src/Discord.Net/Models/Channel.cs index a0191750f..d66d9739b 100644 --- a/src/Discord.Net/Models/Channel.cs +++ b/src/Discord.Net/Models/Channel.cs @@ -8,6 +8,19 @@ namespace Discord { public sealed class Channel : CachedObject { + 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(); } } - + /// Returns the name of this channel. public string Name { get; private set; } /// Returns the topic associated with this channel. @@ -50,13 +63,15 @@ namespace Discord { get { - if (_areMembersStale) - UpdateMembersCache(); - return _members.Select(x => x.Value); + if (Type == ChannelType.Text) + return _members.Values.Where(x => x.Permissions.ReadMessages == true).Select(x => x.User); + else if (Type == ChannelType.Voice) + return Server.Members.Where(x => x.VoiceChannel == this); + else + return Enumerable.Empty(); } } - private Dictionary _members; - private bool _areMembersStale; + private ConcurrentDictionary _members; /// Returns a collection of all messages the client has seen posted in this channel. This collection does not guarantee any ordering. [JsonIgnore] @@ -89,7 +104,13 @@ namespace Discord x.GlobalUser.PrivateChannel = null; }); _permissionOverwrites = _initialPermissionsOverwrites; - _areMembersStale = true; + _members = new ConcurrentDictionary(); + + 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 AddMember(User user) + { + 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); + } - internal void InvalidateMembersCache() + internal ChannelPermissions GetPermissions(User user) { - _areMembersStale = true; + ChannelMember member; + if (_members.TryGetValue(user.Id, out member)) + return member.Permissions; + else + return null; } - private void UpdateMembersCache() + internal void UpdatePermissions() { - if (IsPrivate) - { - _members = new Dictionary() - { - { _client.CurrentUserId, _client.PrivateUser }, - { _recipient.Id.Value, _recipient.Value } - }; - } - else if (Type == ChannelType.Text) + foreach (var pair in _members) { - _members = Server.Members - .Where(x => x.GetPermissions(this)?.ReadMessages ?? false) - .ToDictionary(x => x.Id, x => x); + ChannelMember member = pair.Value; + UpdatePermissions(member.User, member.Permissions); } - else if (Type == ChannelType.Voice) - { - _members = Server.Members - .Where(x => x.VoiceChannel == this) - .ToDictionary(x => x.Id, x => x); - } - _areMembersStale = false; - } - - internal void InvalidatePermissionsCache() - { - UpdateMembersCache(); - foreach (var member in _members) - member.Value.UpdateChannelPermissions(this); } - /*internal void InvalidatePermissionsCache(Role role) + internal void UpdatePermissions(User user) { - _areMembersStale = true; - foreach (var member in role.Members) - member.UpdateChannelPermissions(this); - }*/ - internal void InvalidatePermissionsCache(User user) + ChannelMember member; + if (_members.TryGetValue(user.Id, out member)) + UpdatePermissions(member.User, member.Permissions); + } + private void UpdatePermissions(User user, ChannelPermissions permissions) { - _areMembersStale = true; - user.UpdateChannelPermissions(this); + 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 + { + var channelOverwrites = PermissionOverwrites; + + 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))) + 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; + + if (BitHelper.GetBit(newPermissions, (int)PermissionsBits.ManageRolesOrPermissions)) + newPermissions = mask; //ManageRolesOrPermissions gives all permisions + else if (Type == ChannelType.Text && !BitHelper.GetBit(newPermissions, (int)PermissionsBits.ReadMessages)) + newPermissions = 0; //No read permission on a text channel removes all other permissions + else + 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; diff --git a/src/Discord.Net/Models/Role.cs b/src/Discord.Net/Models/Role.cs index 168ff5123..172317799 100644 --- a/src/Discord.Net/Models/Role.cs +++ b/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; diff --git a/src/Discord.Net/Models/Server.cs b/src/Discord.Net/Models/Server.cs index 99b85feb7..771513087 100644 --- a/src/Discord.Net/Models/Server.cs +++ b/src/Discord.Net/Models/Server.cs @@ -8,7 +8,20 @@ using System.Linq; namespace Discord { public sealed class Server : CachedObject - { + { + private struct ServerMember + { + public readonly User User; + public readonly ServerPermissions Permissions; + + public ServerMember(User user) + { + User = user; + Permissions = new ServerPermissions(); + Permissions.Lock(); + } + } + /// Returns the name of this channel. public string Name { get; private set; } /// Returns the current logged-in user's data for this server. @@ -20,8 +33,6 @@ namespace Discord public DateTime JoinedAt { get; private set; } /// Returns the region for this server (see Regions). public string Region { get; private set; } - /*/// Returns the endpoint for this server's voice server. - internal string VoiceServer { get; set; }*/ /// Returns true if the current user created this server. public bool IsOwner => _client.CurrentUserId == _ownerId; @@ -47,18 +58,18 @@ namespace Discord /// Returns a collection of all channels within this server. [JsonIgnore] public IEnumerable Channels => _channels.Select(x => x.Value); - /// Returns a collection of all channels within this server. + /// Returns a collection of all text channels within this server. [JsonIgnore] public IEnumerable TextChannels => _channels.Select(x => x.Value).Where(x => x.Type == ChannelType.Text); - /// Returns a collection of all channels within this server. + /// Returns a collection of all voice channels within this server. [JsonIgnore] public IEnumerable VoiceChannels => _channels.Select(x => x.Value).Where(x => x.Type == ChannelType.Voice); private ConcurrentDictionary _channels; /// Returns a collection of all users within this server with their server-specific data. [JsonIgnore] - public IEnumerable Members => _members.Select(x => x.Value); - private ConcurrentDictionary _members; + public IEnumerable Members => _members.Select(x => x.Value.User); + private ConcurrentDictionary _members; /// Return the the role representing all users in a server. [JsonIgnore] @@ -75,8 +86,8 @@ namespace Discord //Global Cache _channels = new ConcurrentDictionary(); - _members = new ConcurrentDictionary(); _roles = new ConcurrentDictionary(); + _members = new ConcurrentDictionary(); //Local Cache _bans = new ConcurrentDictionary(); @@ -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) - { - user.AddChannel(channel); - channel.InvalidatePermissionsCache(user); - } + channel.AddMember(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) - { - user.RemoveChannel(channel); - channel.InvalidatePermissionsCache(user); - } + channel.RemoveMember(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); diff --git a/src/Discord.Net/Models/User.cs b/src/Discord.Net/Models/User.cs index 3f8ac6d41..ae6355d01 100644 --- a/src/Discord.Net/Models/User.cs +++ b/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 { internal struct CompositeKey : IEquatable @@ -38,9 +25,6 @@ namespace Discord } internal static string GetAvatarUrl(long userId, string avatarId) => avatarId != null ? Endpoints.UserAvatar(userId, avatarId) : null; - - private ConcurrentDictionary _permissions; - private ServerPermissions _serverPermissions; /// Returns a unique identifier combining this user's id with its server's. internal CompositeKey UniqueId => new CompositeKey(_server.Id ?? 0, Id); @@ -117,10 +101,8 @@ namespace Discord { if (_server.Id != null) { - return _permissions - .Where(x => x.Value.Permissions.ReadMessages) - .Select(x => x.Value.Channel) - .Where(x => x != null); + return Server.Channels + .Where(x => x.GetPermissions(this).ReadMessages); } else { @@ -158,12 +140,6 @@ namespace Discord _roles = new Dictionary(); Status = UserStatus.Offline; - //_channels = new ConcurrentDictionary(); - if (serverId != null) - { - _permissions = new ConcurrentDictionary(); - _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) - { - 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(); - } - } + + _voiceChannel.Id = model.ChannelId; //Allows null + } private void UpdateRoles(IEnumerable roles) { Dictionary newRoles = new Dictionary(); @@ -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() - { - 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 ServerPermissions ServerPermissions => Server.GetPermissions(this); public ChannelPermissions GetPermissions(Channel channel) { if (channel == null) throw new ArgumentNullException(nameof(channel)); - //Return static permissions if this is a private chat - 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); - } + return channel.GetPermissions(this); } public bool HasRole(Role role)