Use nanosleep for non-Windows platforms #6392

hwsmm · 2024-10-18T13:50:14Z

Supersedes Use SDL_DelayNS for lower thread jitter on non-Windows platforms #6391

This PR lets Unix platforms use nanosleep instead of Thread.Sleep to improve on frame pacing. I will test on Android after some time, but I don't have any Apple devices to test this on.

I put UnixNativeSleep in Platform/Linux/Native, but I don't think it's the best fit... Do I need to create a new folder, or is it good as is?

Testing:

Susko3

Hmm, there are a lot of unknowns/variables when trying to implement this in C#: the native library name, definition of struct timespec and EINTR.

Maybe it's better to call SDL3.SDL_DelayNS() and have a comment explaining that it's just a wrapper for nanosleep() on Unix platforms.

Susko3 · 2024-10-18T18:27:57Z

osu.Framework/Platform/Linux/Native/UnixNativeSleep.cs

+            public long Seconds;
+            public long NanoSeconds;


Both of these need to be IntPtr to work properly on 32-bit Unix platforms. Since time_t and long are the underlying types.

I had also thought about this, but do we have any plans to support 32bit systems in future? I've seen many discussions which suggested 32bit won't be supported.

nint also works, but I find this pedantry. 32-bit unices are no longer relevant in $CURRENT_YEAR. We don't even ship all binaries for linux-x86 (ffmpeg is missing).

Android arm32 (armeabi-v7a) might be relevant. If you don't want to use IntPtr/nint here, make this code only work on 64-bit systems by checking Environment.Is64BitProcess.

Susko3 · 2024-10-18T18:29:32Z

osu.Framework/Platform/Linux/Native/UnixNativeSleep.cs

+        [DllImport("libc.so.6", EntryPoint = "nanosleep", CallingConvention = CallingConvention.Cdecl, SetLastError = true)]
+        private static extern int nanosleep_libc6(in TimeSpec duration, out TimeSpec rem);
+
+        private const int interrupt_error = 4;


https://man7.org/linux/man-pages/man3/errno.3.html

The error numbers that correspond to each symbolic name vary
across UNIX systems, and even across different architectures on
Linux. Therefore, numeric values are not included as part of the
list of error names below. The perror(3) and strerror(3)
functions can be used to convert these names to corresponding
textual error messages.

We can't really define a numerical constant for this...

EINTR seems to be 4 on all platforms we support. I need confirmation on Apple devices, though.

I also agree that this is not reliable, but anyway..

As above, this is pedantry. As long as it works it's fine to hardcode. If it doesn't work somewhere, then we'll find out eventually.

I looked at the decompiled SDL3 code in SDL3-CS/native and it's always 4.

hwsmm · 2024-10-19T01:44:18Z

So do we just try using SDL_DelayNS again? I would prefer using SDL_DelayNS for Unix platforms as targeting different libc/arch in C# doesn't sound good for me, but I also agree that using an SDL function in Clock doesn't match well either (might be good enough with the comment as Susko3 mentioned).

peppy · 2024-10-19T10:22:43Z

Where's the testing/benchmarking at? I'd want to see some kind of proof this is beneficial before blindly implementing.

I'd hope this is done by the implementor as to not take time away from the core team.

hwsmm · 2024-10-19T10:33:43Z

You can also see that update thread has (almost) constant 1.2ms frame time on current release, while this implementation keeps it at 1ms. These were taken on Linux, and YMMV as not every distro has same configuration, but it works pretty well on my environment.

Current release	This PR

bdach · 2024-10-21T06:19:37Z

So do we just try using SDL_DelayNS again?

If this diff can't work then I'd rather have nothing than sdl in threading code paths.

Susko3 · 2024-10-21T12:29:17Z

osu.Framework/Platform/Linux/Native/UnixNativeSleep.cs

+        // Android and some platforms don't have version in lib name.
+        [DllImport("c", EntryPoint = "nanosleep", CallingConvention = CallingConvention.Cdecl, SetLastError = true)]
+        private static extern int nanosleep_c(in TimeSpec duration, out TimeSpec rem);
+
+        [DllImport("libc.so.6", EntryPoint = "nanosleep", CallingConvention = CallingConvention.Cdecl, SetLastError = true)]
+        private static extern int nanosleep_libc6(in TimeSpec duration, out TimeSpec rem);


This should just import "libc" and let .NET deal with it.

This avoids having two native functions and the delegate. We can probably assume nanosleep() is always available, so there's no need for the static constructor.

Full proposal implementing the 64-bit check:

diff --git a/osu.Framework/Platform/Linux/Native/UnixNativeSleep.cs b/osu.Framework/Platform/Linux/Native/UnixNativeSleep.cs index d0bf152b3..8418fbff2 100644 --- a/osu.Framework/Platform/Linux/Native/UnixNativeSleep.cs +++ b/osu.Framework/Platform/Linux/Native/UnixNativeSleep.cs @@ -8,6 +8,8 @@ namespace osu.Framework.Platform.Linux.Native { internal class UnixNativeSleep : INativeSleep { + public static bool Available => Environment.Is64BitProcess; + [StructLayout(LayoutKind.Sequential)] public struct TimeSpec { @@ -15,56 +17,13 @@ public struct TimeSpec public long NanoSeconds; } - private delegate int NanoSleepDelegate(in TimeSpec duration, out TimeSpec rem); - - private static readonly NanoSleepDelegate? nanosleep; - - // Android and some platforms don't have version in lib name. - [DllImport("c", EntryPoint = "nanosleep", CallingConvention = CallingConvention.Cdecl, SetLastError = true)] - private static extern int nanosleep_c(in TimeSpec duration, out TimeSpec rem); - - [DllImport("libc.so.6", EntryPoint = "nanosleep", CallingConvention = CallingConvention.Cdecl, SetLastError = true)] - private static extern int nanosleep_libc6(in TimeSpec duration, out TimeSpec rem); + [DllImport("libc", CallingConvention = CallingConvention.Cdecl, SetLastError = true)] + private static extern int nanosleep(in TimeSpec duration, out TimeSpec rem); private const int interrupt_error = 4; - static UnixNativeSleep() - { - TimeSpec test = new TimeSpec - { - Seconds = 0, - NanoSeconds = 1, - }; - - try - { - nanosleep_c(in test, out _); - nanosleep = nanosleep_c; - } - catch - { - } - - if (nanosleep == null) - { - try - { - nanosleep_libc6(in test, out _); - nanosleep = nanosleep_libc6; - } - catch - { - } - } - - // if nanosleep is null at this point, Thread.Sleep should be used. - } - public bool Sleep(TimeSpan duration) { - if (nanosleep == null) - return false; - const int ns_per_second = 1000 * 1000 * 1000; long ns = (long)duration.TotalNanoseconds; diff --git a/osu.Framework/Timing/ThrottledFrameClock.cs b/osu.Framework/Timing/ThrottledFrameClock.cs index 4f827d68c..e0355c33f 100644 --- a/osu.Framework/Timing/ThrottledFrameClock.cs +++ b/osu.Framework/Timing/ThrottledFrameClock.cs @@ -33,13 +33,13 @@ public class ThrottledFrameClock : FramedClock, IDisposable /// </summary> public double TimeSlept { get; private set; } - private readonly INativeSleep nativeSleep; + private readonly INativeSleep? nativeSleep; internal ThrottledFrameClock() { if (RuntimeInfo.OS == RuntimeInfo.Platform.Windows) nativeSleep = new WindowsNativeSleep(); - else + else if (RuntimeInfo.IsUnix && UnixNativeSleep.Available) nativeSleep = new UnixNativeSleep(); } @@ -95,7 +95,7 @@ private double sleepAndUpdateCurrent(double milliseconds) TimeSpan timeSpan = TimeSpan.FromMilliseconds(milliseconds); - if (!nativeSleep.Sleep(timeSpan)) + if (nativeSleep?.Sleep(timeSpan) != true) Thread.Sleep(timeSpan); return (CurrentTime = SourceTime) - before; @@ -103,7 +103,7 @@ private double sleepAndUpdateCurrent(double milliseconds) public void Dispose() { - nativeSleep.Dispose(); + nativeSleep?.Dispose(); } } }

Oh, I noticed your comments after pushing... I think it isn't that bad to use nint if we want to support 32bit Android.

I will revert if anyone thinks otherwise.

sr229 · 2024-10-29T01:10:39Z

You can also see that update thread has (almost) constant 1.2ms frame time on current release, while this implementation keeps it at 1ms. These were taken on Linux, and YMMV as not every distro has same configuration, but it works pretty well on my environment.

Current release This PR

I would prefer if this was benchmarked properly using BenchmarkDotNet to provide reproducible empirical results if there's any benefits. These screenshots won't do.

hwsmm · 2024-10-29T13:31:28Z

0.5ms duration:

Method	Mean	Error	StdDev	Allocated
TestThreadSleep	629.9 ns	7.74 ns	6.86 ns	-
TestNativeSleep	503,293.3 ns	26.55 ns	23.54 ns	-

1ms duration:

Method	Mean	Error	StdDev	Allocated
TestThreadSleep	1.054 ms	0.0001 ms	0.0001 ms	-
TestNativeSleep	1.004 ms	0.0000 ms	0.0000 ms	-

1.5ms duration:

Method	Mean	Error	StdDev	Allocated
TestThreadSleep	1.054 ms	0.0002 ms	0.0002 ms	-
TestNativeSleep	1.503 ms	0.0001 ms	0.0001 ms	-

I think Thread.Sleep is limited to millisecond precision even if TimeSpan is given.

Also, I assumed screenshots were enough to show near 0.00ms frame time stddev, but I agree that this is more clear.

harrislegobrick · 2024-11-19T05:55:28Z

Additional frametime graphs

This PR is essentially Use (high resolution) waitable timers on windows when available #5501 but for all other systems. The benefits listed there should apply to this PR.
I scaled visible_ms_range down in FrameStatisticsDisplay.cs to highlight the decrease in jitter as a smoother graph.

Windows 11

Current release	This PR

Test refactored Windows logic. Change is within margin of error.

Linux (Fedora 41)

Current release	This PR

The threads are sleeping for a more accurate amount of time leading to less gaps in the frametime graph and a smoother graph. The Thread.Sleep() method currently used is limited to integer millisecond level sleeping (as shown in the benchmark above) and a 1kHz loop requires sub millisecond timing. I observed no CPU utilization difference for this PR.

macOS

Current release	This PR

I had a limited amount of time with a MacBook so that's why these graphs are more rough looking than the other two. Even though the graphs look different, the graph for this PR looks smoother overall.

peppy · 2024-11-20T06:48:52Z

Is there a reason why there's (what looks to be) gaps in the old graphs?

harrislegobrick · 2024-11-21T00:00:24Z

Is there a reason why there's (what looks to be) gaps in the old graphs?

The gaps are an artifact of trying to sleep for less than a millisecond returning a tiny value. Applying this patch makes very small sleep times yellow and increases the height to be more visible.

diff --git a/osu.Framework/Graphics/Performance/FrameStatisticsDisplay.cs b/osu.Framework/Graphics/Performance/FrameStatisticsDisplay.cs
index f752c88c5..403138d70 100644
--- a/osu.Framework/Graphics/Performance/FrameStatisticsDisplay.cs
+++ b/osu.Framework/Graphics/Performance/FrameStatisticsDisplay.cs
@@ -38,7 +38,7 @@ internal partial class FrameStatisticsDisplay : Container, IStateful<FrameStatis
         private const int amount_count_steps = 5;
 
         private const int amount_ms_steps = 5;
-        private const float visible_ms_range = 20;
+        private const float visible_ms_range = 0.1f;
         private const float scale = HEIGHT / visible_ms_range;
 
         private const float alpha_when_active = 0.75f;
@@ -490,6 +490,12 @@ private int addArea(FrameStatistics frame, PerformanceCollectionType? frameTimeT
 
             Color4 col = frameTimeType.HasValue ? getColour(frameTimeType.Value) : new Color4(0.1f, 0.1f, 0.1f, 1);
 
+            if (frameTimeType.GetValueOrDefault() == PerformanceCollectionType.Sleep && drawHeight < 10)
+            {
+                col = Color4.Yellow;
+                drawHeight += HEIGHT / 2;
+            }
+
             for (int i = currentHeight - 1; i >= 0; --i)
             {
                 if (drawHeight-- == 0) break;

Linux

`master` (with above patch applied)	This PR (with above patch applied)

peppy · 2024-11-21T07:00:26Z

Right, so it's showing precisely what it should be based on how it was sleeping. All good.

huoyaoyuan · 2024-11-27T16:49:29Z

Interesting. Consider reporting to https://github.com/dotnet/runtime/issues to improve the performance of Thread.Sleep. You may also get feedback around reliability/compatibility there.

Use nanosleep for non-Windows platforms

e97e54f

pull-request-size bot added the size/L label Oct 18, 2024

Better error handling in UnixNativeSleep

a29d4a1

Susko3 reviewed Oct 18, 2024

View reviewed changes

Susko3 suggested changes Oct 21, 2024

View reviewed changes

hwsmm and others added 2 commits October 21, 2024 21:38

Use nint and cleanup detection in UnixNativeSleep

437bd0d

Partly apply Susko3's suggestion for UnixNativeSleep

ba6a084

Add BenchmarkSleep

4d140b9

smoogipoo mentioned this pull request Nov 18, 2024

High CPU usage while minimized #3151

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use nanosleep for non-Windows platforms #6392

Use nanosleep for non-Windows platforms #6392

hwsmm commented Oct 18, 2024 •

edited

Loading

Susko3 left a comment

Susko3 Oct 18, 2024

hwsmm Oct 19, 2024

bdach Oct 21, 2024

Susko3 Oct 21, 2024

Susko3 Oct 18, 2024

hwsmm Oct 19, 2024 •

edited

Loading

bdach Oct 21, 2024

Susko3 Oct 21, 2024

hwsmm commented Oct 19, 2024 •

edited

Loading

peppy commented Oct 19, 2024

hwsmm commented Oct 19, 2024

bdach commented Oct 21, 2024 •

edited

Loading

Susko3 Oct 21, 2024

Susko3 Oct 21, 2024

hwsmm Oct 21, 2024 •

edited

Loading

sr229 commented Oct 29, 2024

hwsmm commented Oct 29, 2024 •

edited

Loading

harrislegobrick commented Nov 19, 2024

peppy commented Nov 20, 2024

harrislegobrick commented Nov 21, 2024

peppy commented Nov 21, 2024

huoyaoyuan commented Nov 27, 2024

Use nanosleep for non-Windows platforms #6392

Are you sure you want to change the base?

Use nanosleep for non-Windows platforms #6392

Conversation

hwsmm commented Oct 18, 2024 • edited Loading

Susko3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwsmm Oct 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwsmm commented Oct 19, 2024 • edited Loading

peppy commented Oct 19, 2024

hwsmm commented Oct 19, 2024

bdach commented Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwsmm Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

sr229 commented Oct 29, 2024

hwsmm commented Oct 29, 2024 • edited Loading

harrislegobrick commented Nov 19, 2024

Additional frametime graphs

peppy commented Nov 20, 2024

harrislegobrick commented Nov 21, 2024

peppy commented Nov 21, 2024

huoyaoyuan commented Nov 27, 2024

hwsmm commented Oct 18, 2024 •

edited

Loading

hwsmm Oct 19, 2024 •

edited

Loading

hwsmm commented Oct 19, 2024 •

edited

Loading

bdach commented Oct 21, 2024 •

edited

Loading

hwsmm Oct 21, 2024 •

edited

Loading

hwsmm commented Oct 29, 2024 •

edited

Loading