-
Notifications
You must be signed in to change notification settings - Fork 423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use nanosleep for non-Windows platforms #6392
base: master
Are you sure you want to change the base?
Changes from 2 commits
e97e54f
a29d4a1
437bd0d
ba6a084
4d140b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence. | ||
// See the LICENCE file in the repository root for full licence text. | ||
|
||
using System; | ||
|
||
namespace osu.Framework.Platform | ||
{ | ||
public interface INativeSleep : IDisposable | ||
{ | ||
bool Sleep(TimeSpan duration); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence. | ||
// See the LICENCE file in the repository root for full licence text. | ||
|
||
using System; | ||
using System.Runtime.InteropServices; | ||
|
||
namespace osu.Framework.Platform.Linux.Native | ||
{ | ||
internal class UnixNativeSleep : INativeSleep | ||
{ | ||
[StructLayout(LayoutKind.Sequential)] | ||
public struct TimeSpec | ||
{ | ||
public long Seconds; | ||
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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should just import This avoids having two native functions and the delegate. We can probably assume There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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();
}
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
private const int interrupt_error = 4; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://man7.org/linux/man-pages/man3/errno.3.html
We can't really define a numerical constant for this... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked at the decompiled SDL3 code in |
||
|
||
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; | ||
|
||
TimeSpec timeSpec = new TimeSpec | ||
{ | ||
Seconds = ns / ns_per_second, | ||
NanoSeconds = ns % ns_per_second, | ||
}; | ||
|
||
int ret; | ||
|
||
while ((ret = nanosleep(in timeSpec, out var remaining)) == -1 && Marshal.GetLastPInvokeError() == interrupt_error) | ||
{ | ||
// The pause can be interrupted by a signal that was delivered to the thread. | ||
// Sleep again with remaining time if it happened. | ||
timeSpec = remaining; | ||
} | ||
|
||
return ret == 0; // Any errors other than interrupt_error should return false. | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence. | ||
// See the LICENCE file in the repository root for full licence text. | ||
|
||
using System; | ||
|
||
namespace osu.Framework.Platform.Windows.Native | ||
{ | ||
internal class WindowsNativeSleep : INativeSleep | ||
{ | ||
private IntPtr waitableTimer; | ||
|
||
public WindowsNativeSleep() | ||
{ | ||
createWaitableTimer(); | ||
} | ||
|
||
private void createWaitableTimer() | ||
{ | ||
try | ||
{ | ||
// Attempt to use CREATE_WAITABLE_TIMER_HIGH_RESOLUTION, only available since Windows 10, version 1803. | ||
waitableTimer = Execution.CreateWaitableTimerEx(IntPtr.Zero, null, | ||
Execution.CreateWaitableTimerFlags.CREATE_WAITABLE_TIMER_MANUAL_RESET | Execution.CreateWaitableTimerFlags.CREATE_WAITABLE_TIMER_HIGH_RESOLUTION, Execution.TIMER_ALL_ACCESS); | ||
|
||
if (waitableTimer == IntPtr.Zero) | ||
{ | ||
// Fall back to a more supported version. This is still far more accurate than Thread.Sleep. | ||
waitableTimer = Execution.CreateWaitableTimerEx(IntPtr.Zero, null, Execution.CreateWaitableTimerFlags.CREATE_WAITABLE_TIMER_MANUAL_RESET, Execution.TIMER_ALL_ACCESS); | ||
} | ||
} | ||
catch | ||
{ | ||
// Any kind of unexpected exception should fall back to Thread.Sleep. | ||
} | ||
} | ||
|
||
public bool Sleep(TimeSpan duration) | ||
{ | ||
if (waitableTimer == IntPtr.Zero) return false; | ||
|
||
// Not sure if we want to fall back to Thread.Sleep on failure here, needs further investigation. | ||
if (Execution.SetWaitableTimerEx(waitableTimer, Execution.CreateFileTime(duration), 0, null, default, IntPtr.Zero, 0)) | ||
{ | ||
Execution.WaitForSingleObject(waitableTimer, Execution.INFINITE); | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
if (waitableTimer != IntPtr.Zero) | ||
Execution.CloseHandle(waitableTimer); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both of these need to be
IntPtr
to work properly on 32-bit Unix platforms. Sincetime_t
andlong
are the underlying types.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Android arm32 (
armeabi-v7a
) might be relevant. If you don't want to useIntPtr
/nint
here, make this code only work on 64-bit systems by checkingEnvironment.Is64BitProcess
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.NET 9+ is only built for distros with 64bit
time_t
on arm32. Usinglong
is more future-proof with the configuration used by .NET.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't matter what configuration .NET is built on, the only thing that matters is which function from libc we import. There's
nanosleep
and_nanosleep64
(or similar).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
nanosleep
is used, the bitness oftimespec
matters, which is decided bytime_t
, which is provided by libc. It can help with keeping consistent with the libc that .NET builds with. That's the point.