From 69f8722e795b0d76b84efa4de52e199e9441fca7 Mon Sep 17 00:00:00 2001 From: mageven <62494521+mageven@users.noreply.github.com> Date: Tue, 23 Mar 2021 00:10:07 +0530 Subject: [PATCH] Fix inconsistencies in progress reporting (#2129) Signal and setup events correctly Eliminate possible races Use a single event Mark volatiles and reduce scope of waithandles Common handler 100ms -> 50ms --- ARMeilleure/Translation/PTC/Ptc.cs | 57 ++++++++++------- .../Translation/PTC/PtcLoadingState.cs | 9 +++ Ryujinx.Graphics.Gpu/GpuContext.cs | 15 +---- Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs | 61 +++++++++++++------ .../Shader/ShaderCacheState.cs | 13 ++++ Ryujinx/Ui/MainWindow.cs | 60 +++++++++--------- 6 files changed, 128 insertions(+), 87 deletions(-) create mode 100644 ARMeilleure/Translation/PTC/PtcLoadingState.cs create mode 100644 Ryujinx.Graphics.Gpu/Shader/ShaderCacheState.cs diff --git a/ARMeilleure/Translation/PTC/Ptc.cs b/ARMeilleure/Translation/PTC/Ptc.cs index c931aaead3..3065757dc4 100644 --- a/ARMeilleure/Translation/PTC/Ptc.cs +++ b/ARMeilleure/Translation/PTC/Ptc.cs @@ -52,14 +52,10 @@ namespace ARMeilleure.Translation.PTC private static readonly ManualResetEvent _waitEvent; - private static readonly AutoResetEvent _loggerEvent; - private static readonly object _lock; private static bool _disposed; - private static volatile int _translateCount; - internal static PtcJumpTable PtcJumpTable { get; private set; } internal static string TitleIdText { get; private set; } @@ -70,9 +66,10 @@ namespace ARMeilleure.Translation.PTC internal static PtcState State { get; private set; } - // Progress update events - public static event Action PtcTranslationStateChanged; - public static event Action PtcTranslationProgressChanged; + // Progress reporting helpers + private static volatile int _translateCount; + private static volatile int _translateTotalCount; + public static event Action PtcStateChanged; static Ptc() { @@ -82,8 +79,6 @@ namespace ARMeilleure.Translation.PTC _waitEvent = new ManualResetEvent(true); - _loggerEvent = new AutoResetEvent(false); - _lock = new object(); _disposed = false; @@ -773,10 +768,20 @@ namespace ARMeilleure.Translation.PTC } _translateCount = 0; + _translateTotalCount = profiledFuncsToTranslate.Count; - ThreadPool.QueueUserWorkItem(TranslationLogger, profiledFuncsToTranslate.Count); + PtcStateChanged?.Invoke(PtcLoadingState.Start, _translateCount, _translateTotalCount); - PtcTranslationStateChanged?.Invoke(true); + using AutoResetEvent progressReportEvent = new AutoResetEvent(false); + + Thread progressReportThread = new Thread(ReportProgress) + { + Name = "Ptc.ProgressReporter", + Priority = ThreadPriority.Lowest, + IsBackground = true + }; + + progressReportThread.Start(progressReportEvent); void TranslateFuncs() { @@ -825,8 +830,12 @@ namespace ARMeilleure.Translation.PTC threads.Clear(); - _loggerEvent.Set(); - PtcTranslationStateChanged?.Invoke(false); + progressReportEvent.Set(); + progressReportThread.Join(); + + PtcStateChanged?.Invoke(PtcLoadingState.Loaded, _translateCount, _translateTotalCount); + + Logger.Info?.Print(LogClass.Ptc, $"{_translateCount} of {_translateTotalCount} functions translated"); PtcJumpTable.Initialize(jumpTable); @@ -838,19 +847,25 @@ namespace ARMeilleure.Translation.PTC preSaveThread.Start(); } - private static void TranslationLogger(object state) + private static void ReportProgress(object state) { - const int refreshRate = 100; // ms + const int refreshRate = 50; // ms - int profiledFuncsToTranslateCount = (int)state; + AutoResetEvent endEvent = (AutoResetEvent)state; + + int count = 0; do { - PtcTranslationProgressChanged?.Invoke(_translateCount, profiledFuncsToTranslateCount); - } - while (!_loggerEvent.WaitOne(refreshRate)); + int newCount = _translateCount; - Logger.Info?.Print(LogClass.Ptc, $"{_translateCount} of {profiledFuncsToTranslateCount} functions translated"); + if (count != newCount) + { + PtcStateChanged?.Invoke(PtcLoadingState.Loading, newCount, _translateTotalCount); + count = newCount; + } + } + while (!endEvent.WaitOne(refreshRate)); } internal static void WriteInfoCodeRelocUnwindInfo(ulong address, ulong guestSize, bool highCq, PtcInfo ptcInfo) @@ -968,8 +983,6 @@ namespace ARMeilleure.Translation.PTC Wait(); _waitEvent.Dispose(); - _loggerEvent.Dispose(); - DisposeMemoryStreams(); } } diff --git a/ARMeilleure/Translation/PTC/PtcLoadingState.cs b/ARMeilleure/Translation/PTC/PtcLoadingState.cs new file mode 100644 index 0000000000..526cf91fb1 --- /dev/null +++ b/ARMeilleure/Translation/PTC/PtcLoadingState.cs @@ -0,0 +1,9 @@ +namespace ARMeilleure.Translation.PTC +{ + public enum PtcLoadingState + { + Start, + Loading, + Loaded + } +} \ No newline at end of file diff --git a/Ryujinx.Graphics.Gpu/GpuContext.cs b/Ryujinx.Graphics.Gpu/GpuContext.cs index 06be6e0579..f131ecc331 100644 --- a/Ryujinx.Graphics.Gpu/GpuContext.cs +++ b/Ryujinx.Graphics.Gpu/GpuContext.cs @@ -80,25 +80,14 @@ namespace Ryujinx.Graphics.Gpu internal Capabilities Capabilities => _caps.Value; /// - /// Signaled when shader cache begins and ends loading. - /// Signals true when loading has started, false when ended. + /// Event for signalling shader cache loading progress. /// - public event Action ShaderCacheStateChanged + public event Action ShaderCacheStateChanged { add => Methods.ShaderCache.ShaderCacheStateChanged += value; remove => Methods.ShaderCache.ShaderCacheStateChanged -= value; } - /// - /// Signaled while shader cache is loading to indicate current progress. - /// Provides current and total number of shaders loaded. - /// - public event Action ShaderCacheProgressChanged - { - add => Methods.ShaderCache.ShaderCacheProgressChanged += value; - remove => Methods.ShaderCache.ShaderCacheProgressChanged -= value; - } - /// /// Creates a new instance of the GPU emulation context. /// diff --git a/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs b/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs index a085936a4a..d99a402b11 100644 --- a/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs +++ b/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs @@ -38,10 +38,9 @@ namespace Ryujinx.Graphics.Gpu.Shader private const ulong ShaderCodeGenVersion = 2088; // Progress reporting helpers - private int _shaderCount; - private readonly AutoResetEvent _progressReportEvent; - public event Action ShaderCacheStateChanged; - public event Action ShaderCacheProgressChanged; + private volatile int _shaderCount; + private volatile int _totalShaderCount; + public event Action ShaderCacheStateChanged; /// /// Creates a new instance of the shader cache. @@ -57,8 +56,6 @@ namespace Ryujinx.Graphics.Gpu.Shader _gpPrograms = new Dictionary>(); _gpProgramsDiskCache = new Dictionary(); _cpProgramsDiskCache = new Dictionary(); - - _progressReportEvent = new AutoResetEvent(false); } /// @@ -85,11 +82,25 @@ namespace Ryujinx.Graphics.Gpu.Shader ReadOnlySpan guestProgramList = _cacheManager.GetGuestProgramList(); - _progressReportEvent.Reset(); - _shaderCount = 0; + using AutoResetEvent progressReportEvent = new AutoResetEvent(false); - ShaderCacheStateChanged?.Invoke(true); - ThreadPool.QueueUserWorkItem(ProgressLogger, guestProgramList.Length); + _shaderCount = 0; + _totalShaderCount = guestProgramList.Length; + + ShaderCacheStateChanged?.Invoke(ShaderCacheState.Start, _shaderCount, _totalShaderCount); + Thread progressReportThread = null; + + if (guestProgramList.Length > 0) + { + progressReportThread = new Thread(ReportProgress) + { + Name = "ShaderCache.ProgressReporter", + Priority = ThreadPriority.Lowest, + IsBackground = true + }; + + progressReportThread.Start(progressReportEvent); + } for (int programIndex = 0; programIndex < guestProgramList.Length; programIndex++) { @@ -318,7 +329,7 @@ namespace Ryujinx.Graphics.Gpu.Shader _gpProgramsDiskCache.Add(key, new ShaderBundle(hostProgram, shaders)); } - _shaderCount = programIndex; + _shaderCount = programIndex + 1; } if (!isReadOnly) @@ -329,26 +340,37 @@ namespace Ryujinx.Graphics.Gpu.Shader _cacheManager.Synchronize(); } - _progressReportEvent.Set(); - ShaderCacheStateChanged?.Invoke(false); + progressReportEvent.Set(); + progressReportThread?.Join(); + + ShaderCacheStateChanged?.Invoke(ShaderCacheState.Loaded, _shaderCount, _totalShaderCount); Logger.Info?.Print(LogClass.Gpu, $"Shader cache loaded {_shaderCount} entries."); } } /// - /// Raises ShaderCacheProgressChanged events periodically. + /// Raises ShaderCacheStateChanged events periodically. /// - private void ProgressLogger(object state) + private void ReportProgress(object state) { - const int refreshRate = 100; // ms + const int refreshRate = 50; // ms + + AutoResetEvent endEvent = (AutoResetEvent)state; + + int count = 0; - int totalCount = (int)state; do { - ShaderCacheProgressChanged?.Invoke(_shaderCount, totalCount); + int newCount = _shaderCount; + + if (count != newCount) + { + ShaderCacheStateChanged?.Invoke(ShaderCacheState.Loading, newCount, _totalShaderCount); + count = newCount; + } } - while (!_progressReportEvent.WaitOne(refreshRate)); + while (!endEvent.WaitOne(refreshRate)); } /// @@ -833,7 +855,6 @@ namespace Ryujinx.Graphics.Gpu.Shader } } - _progressReportEvent?.Dispose(); _cacheManager?.Dispose(); } } diff --git a/Ryujinx.Graphics.Gpu/Shader/ShaderCacheState.cs b/Ryujinx.Graphics.Gpu/Shader/ShaderCacheState.cs new file mode 100644 index 0000000000..623b73d79b --- /dev/null +++ b/Ryujinx.Graphics.Gpu/Shader/ShaderCacheState.cs @@ -0,0 +1,13 @@ +namespace Ryujinx.Graphics.Gpu.Shader +{ + /// Shader cache loading states + public enum ShaderCacheState + { + /// Shader cache started loading + Start, + /// Shader cache is loading + Loading, + /// Shader cache finished loading + Loaded + } +} \ No newline at end of file diff --git a/Ryujinx/Ui/MainWindow.cs b/Ryujinx/Ui/MainWindow.cs index 48e5a5b7a9..1fa52d323f 100644 --- a/Ryujinx/Ui/MainWindow.cs +++ b/Ryujinx/Ui/MainWindow.cs @@ -33,6 +33,9 @@ using System.Threading.Tasks; using GUI = Gtk.Builder.ObjectAttribute; +using PtcLoadingState = ARMeilleure.Translation.PTC.PtcLoadingState; +using ShaderCacheLoadingState = Ryujinx.Graphics.Gpu.Shader.ShaderCacheState; + namespace Ryujinx.Ui { public class MainWindow : Window @@ -105,8 +108,6 @@ namespace Ryujinx.Ui [GUI] Label _loadingStatusLabel; [GUI] ProgressBar _loadingStatusBar; - private string _loadingStatusTitle = ""; - #pragma warning restore CS0649, IDE0044, CS0169 public MainWindow() : this(new Builder("Ryujinx.Ui.MainWindow.glade")) { } @@ -346,43 +347,38 @@ namespace Ryujinx.Ui private void SetupProgressUiHandlers() { - Ptc.PtcTranslationStateChanged -= PtcStatusChanged; - Ptc.PtcTranslationStateChanged += PtcStatusChanged; + Ptc.PtcStateChanged -= ProgressHandler; + Ptc.PtcStateChanged += ProgressHandler; - Ptc.PtcTranslationProgressChanged -= LoadingProgressChanged; - Ptc.PtcTranslationProgressChanged += LoadingProgressChanged; - - _emulationContext.Gpu.ShaderCacheStateChanged -= ShaderCacheStatusChanged; - _emulationContext.Gpu.ShaderCacheStateChanged += ShaderCacheStatusChanged; - - _emulationContext.Gpu.ShaderCacheProgressChanged -= LoadingProgressChanged; - _emulationContext.Gpu.ShaderCacheProgressChanged += LoadingProgressChanged; + _emulationContext.Gpu.ShaderCacheStateChanged -= ProgressHandler; + _emulationContext.Gpu.ShaderCacheStateChanged += ProgressHandler; } - private void ShaderCacheStatusChanged(bool state) + private void ProgressHandler(T state, int current, int total) where T : Enum { - _loadingStatusTitle = "Shaders"; - Application.Invoke(delegate - { - _loadingStatusBar.Visible = _loadingStatusLabel.Visible = state; - }); - } + bool visible; + string label; - private void PtcStatusChanged(bool state) - { - _loadingStatusTitle = "PTC"; - Application.Invoke(delegate + switch (state) { - _loadingStatusBar.Visible = _loadingStatusLabel.Visible = state; - }); - } + case PtcLoadingState ptcState: + visible = ptcState != PtcLoadingState.Loaded; + label = $"PTC : {current}/{total}"; + break; + case ShaderCacheLoadingState shaderCacheState: + visible = shaderCacheState != ShaderCacheLoadingState.Loaded; + label = $"Shaders : {current}/{total}"; + break; + default: + throw new ArgumentException($"Unknown Progress Handler type {typeof(T)}"); + } - private void LoadingProgressChanged(int value, int total) - { Application.Invoke(delegate { - _loadingStatusBar.Fraction = (double)value / total; - _loadingStatusLabel.Text = $"{_loadingStatusTitle} : {value}/{total}"; + _loadingStatusLabel.Text = label; + _loadingStatusBar.Fraction = total > 0 ? (double)current / total : 0; + _loadingStatusBar.Visible = visible; + _loadingStatusLabel.Visible = visible; }); } @@ -464,6 +460,8 @@ namespace Ryujinx.Ui UpdateGraphicsConfig(); + SetupProgressUiHandlers(); + SystemVersion firmwareVersion = _contentManager.GetCurrentFirmwareVersion(); bool isDirectory = Directory.Exists(path); @@ -584,8 +582,6 @@ namespace Ryujinx.Ui _deviceExitStatus.Reset(); - SetupProgressUiHandlers(); - Translator.IsReadyForTranslation.Reset(); #if MACOS_BUILD CreateGameWindow();