fix: fix sorting problems

ChatTwo/Configuration.cs

@@ -1,3 +1,4 @@
+using System.Collections;
 using ChatTwo.Code;
 using ChatTwo.Resources;
 using ChatTwo.Ui;
@@ -166,12 +167,7 @@ internal class Tab
     public uint Unread;
 
     [NonSerialized]
-    public SemaphoreSlim MessagesMutex = new(1, 1);
+    public MessageList Messages = new();
-
-    [NonSerialized]
-    public List<Message> Messages = [];
-    [NonSerialized]
-    public HashSet<Guid> TrackedMessageIds = [];
 
     [NonSerialized]
     public InputChannel? PreviousChannel;
@@ -179,16 +175,6 @@ internal class Tab
     [NonSerialized]
     public Guid Identifier = Guid.NewGuid();
 
-    ~Tab()
-    {
-        MessagesMutex.Dispose();
-    }
-
-    internal bool Contains(Message message)
-    {
-        return TrackedMessageIds.Contains(message.Id);
-    }
-
     internal bool Matches(Message message)
     {
         if (message.ExtraChatChannel != Guid.Empty)
@@ -200,30 +186,16 @@ internal class Tab
                    || sources.HasFlag(message.Code.Source));
     }
 
-    internal void AddMessage(Message message, bool unread = true) {
+    internal void AddMessage(Message message, bool unread = true)
-        if (Contains(message))
+    {
-            return;
+        Messages.AddPrune(message, MessageManager.MessageDisplayLimit);
-
-        MessagesMutex.Wait();
-        TrackedMessageIds.Add(message.Id);
-        Messages.Add(message);
-        while (Messages.Count > MessageManager.MessageDisplayLimit)
-        {
-            TrackedMessageIds.Remove(Messages[0].Id);
-            Messages.RemoveAt(0);
-        }
-        MessagesMutex.Release();
-
         if (unread)
             Unread += 1;
     }
 
     internal void Clear()
     {
-        MessagesMutex.Wait();
         Messages.Clear();
-        TrackedMessageIds.Clear();
-        MessagesMutex.Release();
     }
 
     internal Tab Clone()
@@ -244,6 +216,130 @@ internal class Tab
             InputDisabled = InputDisabled,
         };
     }
+
+    /// <summary>
+    /// MessageList provides an ordered list of messages with duplicate ID
+    /// tracking, sorting and mutex protection.
+    /// </summary>
+    internal class MessageList
+    {
+        private ReaderWriterLock rwl = new();
+
+        private readonly List<Message> messages;
+        private readonly HashSet<Guid> trackedMessageIds;
+
+        public MessageList()
+        {
+            messages = new();
+            trackedMessageIds = new();
+        }
+
+        public MessageList(int initialCapacity)
+        {
+            messages = new(initialCapacity);
+            trackedMessageIds = new(initialCapacity);
+        }
+
+        public void AddPrune(Message message, int max)
+        {
+            rwl.AcquireWriterLock(0);
+            try
+            {
+                AddLocked(message);
+                PruneMaxLocked(max);
+            }
+            finally
+            {
+                rwl.ReleaseWriterLock();
+            }
+        }
+
+        public void AddSortPrune(IEnumerable<Message> messages, int max)
+        {
+            rwl.AcquireWriterLock(0);
+            try
+            {
+                foreach (var message in messages)
+                    AddLocked(message);
+
+                SortLocked();
+                PruneMaxLocked(max);
+            }
+            finally
+            {
+                rwl.ReleaseWriterLock();
+            }
+        }
+
+        private void AddLocked(Message message)
+        {
+            if (trackedMessageIds.Contains(message.Id))
+                return;
+
+            messages.Add(message);
+            trackedMessageIds.Add(message.Id);
+        }
+
+        public void Clear()
+        {
+            rwl.AcquireWriterLock(0);
+            try
+            {
+                messages.Clear();
+                trackedMessageIds.Clear();
+            }
+            finally
+            {
+                rwl.ReleaseWriterLock();
+            }
+        }
+
+        private void SortLocked()
+        {
+            messages.Sort((a, b) => a.Date.CompareTo(b.Date));
+        }
+
+        private void PruneMaxLocked(int max)
+        {
+            while (messages.Count > max)
+            {
+                trackedMessageIds.Remove(messages[0].Id);
+                messages.RemoveAt(0);
+            }
+        }
+
+        /// <summary>
+        /// GetReadOnly returns a read-only list of messages while holding a
+        /// reader lock. The list should be used with a using statement.
+        /// </summary>
+        public RLockedMessageList GetReadOnly(int millisecondsTimeout = 0)
+        {
+            rwl.AcquireReaderLock(millisecondsTimeout);
+            return new RLockedMessageList(rwl, messages);
+        }
+
+        internal class RLockedMessageList(ReaderWriterLock rwl, List<Message> messages) : IReadOnlyList<Message>, IDisposable
+        {
+            public IEnumerator<Message> GetEnumerator()
+            {
+                return messages.GetEnumerator();
+            }
+
+            IEnumerator IEnumerable.GetEnumerator()
+            {
+                return GetEnumerator();
+            }
+
+            public int Count => messages.Count;
+
+            public Message this[int index] => messages[index];
+
+            public void Dispose()
+            {
+                rwl.ReleaseReaderLock();
+            }
+        }
+    }
 }
 
 [Serializable]

ChatTwo/LegacyMessageImporter.cs

@@ -334,7 +334,7 @@ internal class LegacyMessageImporter : IAsyncDisposable
         _database.Dispose();
         _database = null;
 
-        Plugin?.MessageManager.FilterAllTabsAsync(false);
+        Plugin?.MessageManager.FilterAllTabsAsync();
     }
 
     private static Message BsonDocumentToMessage(BsonDocument doc)

ChatTwo/MessageManager.cs

@@ -20,10 +20,20 @@ internal class MessageManager : IAsyncDisposable
 
     private Dictionary<ChatType, NameFormatting> Formats { get; } = new();
     private ulong LastContentId { get; set; }
-
-    private ConcurrentQueue<PendingMessage> Pending { get; } = new();
     private int LastMessageIndex { get; set; }
 
+    // Messages go into the PendingSync queue first, which will be consumed one
+    // at a time in the main thread. The only processing that will be done is
+    // to fetch the ContentId for the message if the int is not 0. Fetching
+    // content ID must happen in the next tick AFTER receiving the message via
+    // the event listener, because when the event handler is called the message
+    // isn't in the log yet.
+    //
+    // After that, the message is enqueued in the PendingAsync queue, which will
+    // be consumed in a separate thread and perform more processing (emotes,
+    // URLs) as well as inserting the message into the database.
+    private Queue<(int, PendingMessage pendingMessage)> PendingSync { get; } = new();
+    private ConcurrentQueue<PendingMessage> PendingAsync { get; } = new();
     private readonly Thread PendingMessageThread;
     private readonly CancellationTokenSource PendingThreadCancellationToken = new();
 
@@ -45,14 +55,14 @@ internal class MessageManager : IAsyncDisposable
         PendingMessageThread.Start();
 
         Plugin.ChatGui.ChatMessageUnhandled += ChatMessage;
-        Plugin.Framework.Update += UpdateReceiver;
+        Plugin.Framework.Update += OnFrameworkUpdate;
         Plugin.ClientState.Logout += Logout;
     }
 
     public async ValueTask DisposeAsync()
     {
         Plugin.ClientState.Logout -= Logout;
-        Plugin.Framework.Update -= UpdateReceiver;
+        Plugin.Framework.Update -= OnFrameworkUpdate;
         Plugin.ChatGui.ChatMessageUnhandled -= ChatMessage;
 
         await PendingThreadCancellationToken.CancelAsync();
@@ -78,20 +88,30 @@ internal class MessageManager : IAsyncDisposable
     private void Logout()
     {
         LastContentId = 0;
+        LastMessageIndex = 0;
     }
 
-    private void UpdateReceiver(IFramework framework)
+    private void OnFrameworkUpdate(IFramework framework)
     {
         var contentId = Plugin.ClientState.LocalContentId;
         if (contentId != 0)
             LastContentId = contentId;
+
+        while (true)
+        {
+            if (!PendingSync.TryDequeue(out var pending)) return;
+
+            if (pending.Item1 > 0)
+                pending.Item2.ContentId = Plugin.Functions.Chat.GetContentIdForEntry(pending.Item1);
+            PendingAsync.Enqueue(pending.Item2);
+        }
     }
 
     private void ProcessPendingMessages(CancellationToken token)
     {
         while (!token.IsCancellationRequested)
         {
-            if (Pending.TryDequeue(out var pendingMessage))
+            if (PendingAsync.TryDequeue(out var pendingMessage))
             {
                 try
                 {
@@ -115,16 +135,24 @@ internal class MessageManager : IAsyncDisposable
             tab.Clear();
     }
 
-    internal void FilterAllTabs(bool unread = true)
+    internal void FilterAllTabs()
     {
         DateTimeOffset? since = null;
         if (!Plugin.Config.FilterIncludePreviousSessions)
             since = Plugin.GameStarted;
 
         var messages = Store.GetMostRecentMessages(CurrentContentId, since);
+
+        // We store the pending messages to be added to the chat log in a
+        // temporary list, and apply them all at once after filtering.
+        var pendingTabs = Plugin.Config.Tabs.Select(tab => (tab, new List<Message>())).ToList();
         foreach (var message in messages)
-            foreach (var tab in Plugin.Config.Tabs.Where(tab => tab.Matches(message)))
+            foreach (var (_, pendingMessages) in pendingTabs.Where(ptab => ptab.Item1.Matches(message)))
-                tab.AddMessage(message, unread);
+                pendingMessages.Add(message);
+
+        // Apply the messages to the chat log in one go.
+        foreach (var (tab, pendingMessages) in pendingTabs)
+            tab.Messages.AddSortPrune(pendingMessages, MessageDisplayLimit);
 
         if (!messages.DidError) return;
 
@@ -141,14 +169,14 @@ internal class MessageManager : IAsyncDisposable
         }
     }
 
-    internal void FilterAllTabsAsync(bool unread = true)
+    internal void FilterAllTabsAsync()
     {
         Task.Run(() =>
         {
             var stopwatch = Stopwatch.StartNew();
             try
             {
-                FilterAllTabs(unread);
+                FilterAllTabs();
             }
             catch (Exception ex)
             {
@@ -182,12 +210,10 @@ internal class MessageManager : IAsyncDisposable
         // content ID is used to show "invite to party" buttons in the context
         // menu.
         var idx = Plugin.Functions.GetCurrentChatLogEntryIndex();
-        var shouldGetContentId = false;
+        if (idx <= LastMessageIndex)
-        if (idx > LastMessageIndex)
+            idx = 0;
-        {
+        else
             LastMessageIndex = idx;
-            shouldGetContentId = true;
-        }
 
         // You can't call GetContentIdForEntry in the same framework tick
         // that you received the message, or you just get null.
@@ -196,12 +222,7 @@ internal class MessageManager : IAsyncDisposable
         // because of this. We used to only delay messages that we wanted to
         // fetch a content ID for, but this results in out-of-order messages
         // occasionally.
-        Plugin.Framework.RunOnTick(() =>
+        PendingSync.Enqueue((idx - 1, pendingMessage));
-        {
-            if (shouldGetContentId)
-                pendingMessage.ContentId = Plugin.Functions.Chat.GetContentIdForEntry(idx - 1);
-            Pending.Enqueue(pendingMessage);
-        });
     }
 
     private void ProcessMessage(PendingMessage pendingMessage)

ChatTwo/Plugin.cs

@@ -118,7 +118,7 @@ public sealed class Plugin : IDalamudPlugin
             Commands.Initialise();
 
             if (Interface.Reason is not PluginLoadReason.Boot)
-                MessageManager.FilterAllTabsAsync(false);
+                MessageManager.FilterAllTabsAsync();
 
             Framework.Update += FrameworkUpdate;
             Interface.UiBuilder.Draw += Draw;

ChatTwo/Ui/ChatLogWindow.cs

@@ -141,7 +141,7 @@ public sealed class ChatLogWindow : Window
 
     private void Login()
     {
-        Plugin.MessageManager.FilterAllTabsAsync(false);
+        Plugin.MessageManager.FilterAllTabsAsync();
     }
 
     private void Activated(ChatActivatedArgs args)
@@ -923,7 +923,8 @@ public sealed class ChatLogWindow : Window
     {
         try
         {
-            tab.MessagesMutex.Wait();
+            // This may produce ApplicationException which is catched below.
+            using var messages = tab.Messages.GetReadOnly(3);
 
             var reset = false;
             if (LastResize is { IsRunning: true, Elapsed.TotalSeconds: > 0.25 })
@@ -939,10 +940,10 @@ public sealed class ChatLogWindow : Window
             var sameCount = 0;
 
             var maxLines = Plugin.Config.MaxLinesToRender;
-            var startLine = tab.Messages.Count > maxLines ? tab.Messages.Count - maxLines : 0;
+            var startLine = messages.Count > maxLines ? messages.Count - maxLines : 0;
-            for (var i = startLine; i < tab.Messages.Count; i++)
+            for (var i = startLine; i < messages.Count; i++)
             {
-                var message = tab.Messages[i];
+                var message = messages[i];
                 if (reset)
                 {
                     message.Height[tab.Identifier] = null;
@@ -957,7 +958,7 @@ public sealed class ChatLogWindow : Window
                     {
                         sameCount += 1;
                         message.IsVisible[tab.Identifier] = false;
-                        if (i != tab.Messages.Count - 1)
+                        if (i != messages.Count - 1)
                             continue;
                     }
 
@@ -974,7 +975,7 @@ public sealed class ChatLogWindow : Window
                     }
 
                     lastMessageHash = messageHash;
-                    if (same && i == tab.Messages.Count - 1)
+                    if (same && i == messages.Count - 1)
                         continue;
                 }
 
@@ -987,7 +988,7 @@ public sealed class ChatLogWindow : Window
                 // the top of the current message.
                 if (i > 0)
                 {
-                    var prevMessage = tab.Messages[i - 1];
+                    var prevMessage = messages[i - 1];
 
                     // TODO: TryGetValue isn't always true for some strange reason
                     // This should be looked into, because default will be null for the prevHeight in that case
@@ -1041,13 +1042,21 @@ public sealed class ChatLogWindow : Window
                     {
                         if (!Plugin.Config.HideSameTimestamps || timestamp != lastTimestamp)
                         {
-                            ImGui.TextUnformatted(timestamp);
                             lastTimestamp = timestamp;
+                            ImGui.TextUnformatted(timestamp);
+                            // We use an IsItemHovered() check here instead of
+                            // just calling SetTooltip() to avoid computing the
+                            // tooltip string for all visible items on every
+                            // frame.
+                            if (ImGui.IsItemHovered())
+                                ImGui.SetTooltip(message.Date.ToLocalTime().ToString("F"));
                         }
                         else
+                        {
                             // Avoids rendering issues caused by emojis in
                             // message content.
                             ImGui.TextUnformatted("");
+                        }
                     }
                     else
                     {
@@ -1075,9 +1084,14 @@ public sealed class ChatLogWindow : Window
                 message.IsVisible[tab.Identifier] = ImGui.IsItemVisible();
             }
         }
-        finally
+        catch (ApplicationException)
         {
-            tab.MessagesMutex.Release();
+            // We couldn't get a reader lock on messages within 3ms, so
+            // don't draw anything (and don't log a warning either).
+        }
+        catch (Exception ex)
+        {
+            Plugin.Log.Warning(ex, "Error drawing chat log");
         }
     }
 

ChatTwo/Ui/Settings.cs

@@ -163,7 +163,7 @@ public sealed class SettingsWindow : Window
         // commit any changes that cause a crash
         Plugin.DeferredSaveFrames = 60;
         Plugin.MessageManager.ClearAllTabs();
-        Plugin.MessageManager.FilterAllTabsAsync(false);
+        Plugin.MessageManager.FilterAllTabsAsync();
 
         if (fontChanged || fontSizeChanged)
             Plugin.FontManager.BuildFonts();

ChatTwo/Ui/SettingsTabs/Database.cs

@@ -153,10 +153,10 @@ internal sealed class Database : ISettingsTab
         if (ImGuiUtil.CtrlShiftButton("Perform maintenance", "Ctrl+Shift: MessageManager.Store.PerformMaintenance()"))
             Plugin.MessageManager.Store.PerformMaintenance();
 
-        if (ImGuiUtil.CtrlShiftButton("Reload messages from database", "Ctrl+Shift: MessageManager.FilterAllTabs(false)"))
+        if (ImGuiUtil.CtrlShiftButton("Reload messages from database", "Ctrl+Shift: MessageManager.FilterAllTabs()"))
         {
             Plugin.MessageManager.ClearAllTabs();
-            Plugin.MessageManager.FilterAllTabsAsync(false);
+            Plugin.MessageManager.FilterAllTabsAsync();
         }
 
         if (ImGuiUtil.CtrlShiftButton("Inject 10,000 messages", "Ctrl+Shift: creates 10,000 unique messages (async)"))
@@ -232,7 +232,7 @@ internal sealed class Database : ISettingsTab
         {
             stopwatch = Stopwatch.StartNew();
             // Intentionally synchronous
-            Plugin.MessageManager.FilterAllTabs(false);
+            Plugin.MessageManager.FilterAllTabs();
             elapsedTicks = stopwatch.ElapsedTicks;
             stopwatch.Stop();
             Plugin.Log.Info($"Fetched and filtered all tabs in {elapsedTicks} ticks ({elapsedTicks / TimeSpan.TicksPerMillisecond}ms)");