From e9ae8362f5eddf1cb12ce940bab9cdc1ca9ebae3 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Sat, 25 May 2024 00:30:44 +1000 Subject: [PATCH 1/2] fix: fix sorting problems --- ChatTwo/Configuration.cs | 162 ++++++++++++++++++++++------ ChatTwo/LegacyMessageImporter.cs | 2 +- ChatTwo/MessageManager.cs | 65 +++++++---- ChatTwo/Plugin.cs | 2 +- ChatTwo/Ui/ChatLogWindow.cs | 36 +++++-- ChatTwo/Ui/Settings.cs | 2 +- ChatTwo/Ui/SettingsTabs/Database.cs | 6 +- 7 files changed, 203 insertions(+), 72 deletions(-) diff --git a/ChatTwo/Configuration.cs b/ChatTwo/Configuration.cs index 9c280b4..bd36048 100755 --- a/ChatTwo/Configuration.cs +++ b/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); - - [NonSerialized] - public List Messages = []; - [NonSerialized] - public HashSet TrackedMessageIds = []; + public MessageList Messages = new(); [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) { - if (Contains(message)) - return; - - MessagesMutex.Wait(); - TrackedMessageIds.Add(message.Id); - Messages.Add(message); - while (Messages.Count > MessageManager.MessageDisplayLimit) - { - TrackedMessageIds.Remove(Messages[0].Id); - Messages.RemoveAt(0); - } - MessagesMutex.Release(); - + internal void AddMessage(Message message, bool unread = true) + { + Messages.AddPrune(message, MessageManager.MessageDisplayLimit); 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, }; } + + /// + /// MessageList provides an ordered list of messages with duplicate ID + /// tracking, sorting and mutex protection. + /// + internal class MessageList + { + private ReaderWriterLock rwl = new(); + + private readonly List messages; + private readonly HashSet 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 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); + } + } + + /// + /// GetReadOnly returns a read-only list of messages while holding a + /// reader lock. The list should be used with a using statement. + /// + public RLockedMessageList GetReadOnly(int millisecondsTimeout = 0) + { + rwl.AcquireReaderLock(millisecondsTimeout); + return new RLockedMessageList(rwl, messages); + } + + internal class RLockedMessageList(ReaderWriterLock rwl, List messages) : IReadOnlyList, IDisposable + { + public IEnumerator 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] diff --git a/ChatTwo/LegacyMessageImporter.cs b/ChatTwo/LegacyMessageImporter.cs index f716438..4358565 100644 --- a/ChatTwo/LegacyMessageImporter.cs +++ b/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) diff --git a/ChatTwo/MessageManager.cs b/ChatTwo/MessageManager.cs index 6454c74..9704c79 100644 --- a/ChatTwo/MessageManager.cs +++ b/ChatTwo/MessageManager.cs @@ -20,10 +20,20 @@ internal class MessageManager : IAsyncDisposable private Dictionary Formats { get; } = new(); private ulong LastContentId { get; set; } - - private ConcurrentQueue 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 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())).ToList(); foreach (var message in messages) - foreach (var tab in Plugin.Config.Tabs.Where(tab => tab.Matches(message))) - tab.AddMessage(message, unread); + foreach (var (_, pendingMessages) in pendingTabs.Where(ptab => ptab.Item1.Matches(message))) + 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(() => - { - if (shouldGetContentId) - pendingMessage.ContentId = Plugin.Functions.Chat.GetContentIdForEntry(idx - 1); - Pending.Enqueue(pendingMessage); - }); + PendingSync.Enqueue((idx - 1, pendingMessage)); } private void ProcessMessage(PendingMessage pendingMessage) diff --git a/ChatTwo/Plugin.cs b/ChatTwo/Plugin.cs index 2e7fe1f..d26021d 100755 --- a/ChatTwo/Plugin.cs +++ b/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; diff --git a/ChatTwo/Ui/ChatLogWindow.cs b/ChatTwo/Ui/ChatLogWindow.cs index 3d00dc2..1e6eca1 100644 --- a/ChatTwo/Ui/ChatLogWindow.cs +++ b/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; - for (var i = startLine; i < tab.Messages.Count; i++) + var startLine = messages.Count > maxLines ? messages.Count - maxLines : 0; + 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"); } } diff --git a/ChatTwo/Ui/Settings.cs b/ChatTwo/Ui/Settings.cs index d19b17c..515895c 100755 --- a/ChatTwo/Ui/Settings.cs +++ b/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(); diff --git a/ChatTwo/Ui/SettingsTabs/Database.cs b/ChatTwo/Ui/SettingsTabs/Database.cs index f12ceb4..92410ea 100755 --- a/ChatTwo/Ui/SettingsTabs/Database.cs +++ b/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)"); From 748a963acc9c8662461b069acf2f117d6a4d2c54 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Sun, 26 May 2024 00:33:21 +1000 Subject: [PATCH 2/2] Use ContentIdResolver hook --- ChatTwo/GameFunctions/GameFunctions.cs | 5 --- ChatTwo/MessageManager.cs | 61 +++++++++++++------------- ChatTwo/Plugin.cs | 2 + 3 files changed, 33 insertions(+), 35 deletions(-) diff --git a/ChatTwo/GameFunctions/GameFunctions.cs b/ChatTwo/GameFunctions/GameFunctions.cs index f5d3bb6..c0a2730 100755 --- a/ChatTwo/GameFunctions/GameFunctions.cs +++ b/ChatTwo/GameFunctions/GameFunctions.cs @@ -69,11 +69,6 @@ internal unsafe class GameFunctions : IDisposable return (nint) infoModule->GetInfoProxyById(idx); } - internal int GetCurrentChatLogEntryIndex() - { - return Framework.Instance()->GetUiModule()->GetRaptureLogModule()->LogModule.LogMessageCount; - } - internal void SendFriendRequest(string name, ushort world) { ListCommand(name, world, "friendlist"); diff --git a/ChatTwo/MessageManager.cs b/ChatTwo/MessageManager.cs index 9704c79..e0e882e 100644 --- a/ChatTwo/MessageManager.cs +++ b/ChatTwo/MessageManager.cs @@ -5,8 +5,11 @@ using ChatTwo.Resources; using ChatTwo.Util; using Dalamud.Game.Text; using Dalamud.Game.Text.SeStringHandling; +using Dalamud.Hooking; using Dalamud.Interface.Internal.Notifications; using Dalamud.Plugin.Services; +using Dalamud.Utility.Signatures; +using FFXIVClientStructs.FFXIV.Client.UI.Misc; using Lumina.Excel.GeneratedSheets; namespace ChatTwo; @@ -20,23 +23,25 @@ internal class MessageManager : IAsyncDisposable private Dictionary Formats { get; } = new(); private ulong LastContentId { get; set; } - 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. + // at a time in the main thread. This is to delay the async processing until + // after we've received the content ID from the ContentIdResolver hook. // // 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 Queue PendingSync { get; } = new(); private ConcurrentQueue PendingAsync { get; } = new(); private readonly Thread PendingMessageThread; private readonly CancellationTokenSource PendingThreadCancellationToken = new(); + // TODO: replace with CS version + private unsafe delegate void ContentIdResolverDelegate(RaptureLogModule* param1, ulong param2, int param3, short param4, short param5); + + [Signature("4C 8B D1 48 8B 89 ?? ?? ?? ?? 48 85 C9", DetourName = nameof(ContentIdResolver))] + private Hook? ContentIdResolverHook { get; init; } + internal ulong CurrentContentId { get @@ -54,6 +59,7 @@ internal class MessageManager : IAsyncDisposable PendingMessageThread = new Thread(() => ProcessPendingMessages(PendingThreadCancellationToken.Token)); PendingMessageThread.Start(); + ContentIdResolverHook?.Enable(); Plugin.ChatGui.ChatMessageUnhandled += ChatMessage; Plugin.Framework.Update += OnFrameworkUpdate; Plugin.ClientState.Logout += Logout; @@ -61,6 +67,7 @@ internal class MessageManager : IAsyncDisposable public async ValueTask DisposeAsync() { + ContentIdResolverHook?.Dispose(); Plugin.ClientState.Logout -= Logout; Plugin.Framework.Update -= OnFrameworkUpdate; Plugin.ChatGui.ChatMessageUnhandled -= ChatMessage; @@ -88,7 +95,6 @@ internal class MessageManager : IAsyncDisposable private void Logout() { LastContentId = 0; - LastMessageIndex = 0; } private void OnFrameworkUpdate(IFramework framework) @@ -97,13 +103,12 @@ internal class MessageManager : IAsyncDisposable if (contentId != 0) LastContentId = contentId; + // Drain the PendingSync queue into the PendingAsync queue. 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); + if (!PendingSync.TryDequeue(out var pending)) + return; + PendingAsync.Enqueue(pending); } } @@ -205,24 +210,20 @@ internal class MessageManager : IAsyncDisposable // Update colour codes. GlobalParametersCache.Refresh(); - // If the message was rendered in the vanilla chat log window it has an - // index, and we can use that to get the sender's content ID. The - // content ID is used to show "invite to party" buttons in the context - // menu. - var idx = Plugin.Functions.GetCurrentChatLogEntryIndex(); - if (idx <= LastMessageIndex) - idx = 0; - else - LastMessageIndex = idx; + // We delay messages to be handed off to the async processing thread + // in the next tick, otherwise we can't get the content ID from the hook + // below. + PendingSync.Enqueue(pendingMessage); + } - // You can't call GetContentIdForEntry in the same framework tick - // that you received the message, or you just get null. - // - // We delay all messages to be enqueued in the next framework tick - // 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. - PendingSync.Enqueue((idx - 1, pendingMessage)); + // This hook is called immediately after receiving a message with the + // message's content ID. If multiple messages are received in the same tick, + // this will be called for each message immediately after ChatMessage is + // called for each message. + private unsafe void ContentIdResolver(RaptureLogModule* param1, ulong param2, int param3, short param4, short param5) + { + PendingSync.Last().ContentId = param2; + ContentIdResolverHook?.Original(param1, param2, param3, param4, param5); } private void ProcessMessage(PendingMessage pendingMessage) diff --git a/ChatTwo/Plugin.cs b/ChatTwo/Plugin.cs index d26021d..dadca42 100755 --- a/ChatTwo/Plugin.cs +++ b/ChatTwo/Plugin.cs @@ -5,8 +5,10 @@ using ChatTwo.Ipc; using ChatTwo.Resources; using ChatTwo.Ui; using ChatTwo.Util; +using Dalamud.Game; using Dalamud.Game.ClientState.Conditions; using Dalamud.Game.ClientState.Objects; +using Dalamud.Hooking; using Dalamud.Interface.Windowing; using Dalamud.IoC; using Dalamud.Plugin;