Merge fix(scripting-mono-v2): allow scheduling from non-main threads …

…(pr-2423)

 - c7b9c21 fix(scripting-mono-v2): allow scheduling from non-main threads

code/client/clrcore-v2/BaseScript.cs

@@ -278,7 +278,6 @@ internal void RegisterKeyMap(string command, string description, string inputMap
 #else
 			if (inputMapper != null && inputParameter != null)
 			{
-				Debug.WriteLine(command);
 				Native.CoreNatives.RegisterKeyMapping(command, description, inputMapper, inputParameter);
 			}
 			m_commands.Add(new KeyValuePair<int, DynFunc>(ReferenceFunctionManager.CreateCommand(command, dynFunc, false), dynFunc));

code/client/clrcore-v2/Coroutine/Scheduler.cs

@@ -43,6 +43,7 @@ public static void Schedule(Action coroutine)
 				lock (s_nextFrame)
 				{
 					s_nextFrame.Add(coroutine);
+					ScriptInterface.RequestTickNextFrame();
 				}
 			}
 			else
@@ -72,14 +73,30 @@ public static void Schedule(Action coroutine, TimePoint time)
 				lock (s_queue)
 				{
 					// linear ordered insert, performance improvement might be a binary tree (i.e.: priority queue)
-					for (var it = s_queue.First; it != null; it = it.Next)
+					var it = s_queue.First;
+					if (it != null)
 					{
+						// if added to the front, we'll also need to request for a tick/call-in
 						if (time < it.Value.Item1)
 						{
-							s_queue.AddBefore(it, new Tuple<ulong, Action>(time, coroutine));
+							s_queue.AddFirst(new Tuple<ulong, Action>(time, coroutine));
+							ScriptInterface.RequestTick(time);
+
 							return;
 						}
+
+						// check next
+						for (it = it.Next; it != null; it = it.Next)
+						{
+							if (time < it.Value.Item1)
+							{
+								s_queue.AddBefore(it, new Tuple<ulong, Action>(time, coroutine));
+								return;
+							}
+						}
 					}
+					else
+						ScriptInterface.RequestTick(time);
 
 					s_queue.AddLast(new Tuple<ulong, Action>(time, coroutine));
 				}
@@ -168,7 +185,10 @@ internal static void Update()
 						}
 					}
 					else
+					{
+						ScriptInterface.RequestTick(curIt.Value.Item1);
 						return;
+					}
 				}
 			}
 		}

code/client/clrcore-v2/Interop/Types/ScriptSharedData.cs

@@ -0,0 +1,19 @@
+using System.Runtime.InteropServices;
+using System.Threading;
+
+namespace CitizenFX.Core
+{
+	[StructLayout(LayoutKind.Explicit)]
+	internal struct ScriptSharedData
+	{
+		/// <summary>
+		/// Next time when our host needs to call in again
+		/// </summary>
+		[FieldOffset(0)] public ulong m_scheduledTime;
+
+		/// <summary>
+		/// Same as <see cref="m_scheduledTime"/> but used in methods like <see cref="Interlocked.CompareExchange(ref long, long, long)" /> who miss a <see cref="ulong"/> overload.
+		/// </summary>
+		[FieldOffset(0)] public long m_scheduledTimeAsLong;
+	};
+}

code/client/clrcore-v2/ScriptInterface.cs

@@ -1,8 +1,8 @@
 using System;
 using System.Diagnostics.CodeAnalysis;
-using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Security;
+using System.Threading;
 
 #if IS_FXSERVER
 using ContextType = CitizenFX.Core.fxScriptContext;
@@ -12,8 +12,6 @@
 
 /*
 * Notes while working on this environment:
-*  - Scheduling: any function that can potentially add tasks to the C#'s scheduler needs to return the time
-*    of when it needs to be activated again, which then needs to be scheduled in the core scheduler (bookmark).
 */
 
 namespace CitizenFX.Core
@@ -25,6 +23,8 @@ internal static class ScriptInterface
 		internal static string ResourceName { get; private set; }
 		internal static CString CResourceName { get; private set; }
 
+		private static unsafe ScriptSharedData* s_sharedData;
+
 		#region Callable from C#
 
 		[SecurityCritical, MethodImpl(MethodImplOptions.InternalCall)]
@@ -71,16 +71,37 @@ internal static class ScriptInterface
 		[SecurityCritical]
 		internal static unsafe bool ReadAssembly(string file, out byte[] assembly, out byte[] symbols) => ReadAssembly(s_runtime, file, out assembly, out symbols);
 
+		/// <summary>
+		/// Schedule a call-in at the given time, uses CAS to make sure we only overwrite if the given time is earlier than the stored one.
+		/// </summary>
+		/// <param name="time">Next time to request a call in</param>
+		[SecuritySafeCritical]
+		internal static unsafe void RequestTick(ulong time)
+		{			
+			ulong prevTime = (ulong)Interlocked.Read(ref s_sharedData->m_scheduledTimeAsLong);
+			while (time < prevTime)
+			{
+				prevTime = (ulong)Interlocked.CompareExchange(ref s_sharedData->m_scheduledTimeAsLong, (long)time, (long)prevTime);
+			}
+		}
+
+		/// <summary>
+		/// Schedule a call-in for the next frame.
+		/// </summary>
+		[SecuritySafeCritical]
+		public static unsafe void RequestTickNextFrame() => s_sharedData->m_scheduledTime = 0UL; // 64 bit read/writes – while aligned – are atomic on 64 bit machines
+
 		#endregion
 
 		#region Called by Native
 		[SecurityCritical, SuppressMessage("System.Diagnostics.CodeAnalysis", "IDE0051", Justification = "Called by host")]
-		internal static void Initialize(string resourceName, UIntPtr runtime, int instanceId)
+		private static unsafe void Initialize(string resourceName, UIntPtr runtime, int instanceId, ScriptSharedData* sharedData)
 		{
 			s_runtime = runtime;
 			InstanceId = instanceId;
 			ResourceName = resourceName;
 			CResourceName = resourceName;
+			s_sharedData = sharedData;
 
 			Resource.Current = new Resource(resourceName);
 			Debug.Initialize(resourceName);
@@ -93,7 +114,7 @@ internal static void Initialize(string resourceName, UIntPtr runtime, int instan
 		}
 
 		[SecurityCritical, SuppressMessage("System.Diagnostics.CodeAnalysis", "IDE0051", Justification = "Called by host")]
-		internal static ulong Tick(ulong hostTime, bool profiling)
+		internal static void Tick(ulong hostTime, bool profiling)
 		{
 			Scheduler.CurrentTime = (TimePoint)hostTime;
 			Profiler.IsProfiling = profiling;
@@ -107,12 +128,10 @@ internal static ulong Tick(ulong hostTime, bool profiling)
 			{
 				Debug.PrintError(e, "Tick()");
 			}
-
-			return Scheduler.NextTaskTime();
 		}
 
 		[SecurityCritical, SuppressMessage("System.Diagnostics.CodeAnalysis", "IDE0051", Justification = "Called by host")]
-		internal static unsafe ulong TriggerEvent(string eventName, byte* argsSerialized, int serializedSize, string sourceString, ulong hostTime, bool profiling)
+		internal static unsafe void TriggerEvent(string eventName, byte* argsSerialized, int serializedSize, string sourceString, ulong hostTime, bool profiling)
 		{
 			Scheduler.CurrentTime = (TimePoint)hostTime;
 			Profiler.IsProfiling = profiling;
@@ -136,30 +155,24 @@ internal static unsafe ulong TriggerEvent(string eventName, byte* argsSerialized
 					EventsManager.IncomingEvent(eventName, sourceString, origin, argsSerialized, serializedSize, args);
 				}
 			}
-
-			return Scheduler.NextTaskTime();
 		}
 
 		[SecurityCritical, SuppressMessage("System.Diagnostics.CodeAnalysis", "IDE0051", Justification = "Called by host")]
-		internal static unsafe ulong LoadAssembly(string name, ulong hostTime, bool profiling)
+		internal static unsafe void LoadAssembly(string name, ulong hostTime, bool profiling)
 		{
 			Scheduler.CurrentTime = (TimePoint)hostTime;
 			Profiler.IsProfiling = profiling;
 
 			ScriptManager.LoadAssembly(name, true);
-
-			return Scheduler.NextTaskTime();
 		}
 
 		[SecurityCritical, SuppressMessage("System.Diagnostics.CodeAnalysis", "IDE0051", Justification = "Called by host")]
-		internal static unsafe ulong CallRef(int refIndex, byte* argsSerialized, uint argsSize, out IntPtr retvalSerialized, out uint retvalSize, ulong hostTime, bool profiling)
+		internal static unsafe void CallRef(int refIndex, byte* argsSerialized, uint argsSize, out IntPtr retvalSerialized, out uint retvalSize, ulong hostTime, bool profiling)
 		{
 			Scheduler.CurrentTime = (TimePoint)hostTime;
 			Profiler.IsProfiling = profiling;
 
 			ReferenceFunctionManager.IncomingCall(refIndex, argsSerialized, argsSize, out retvalSerialized, out retvalSize);
-
-			return Scheduler.NextTaskTime();
 		}
 
 		[SecurityCritical, SuppressMessage("System.Diagnostics.CodeAnalysis", "IDE0051", Justification = "Called by host")]

code/components/citizen-scripting-mono-v2/include/MonoScriptRuntime.h

@@ -18,10 +18,11 @@
 #include <om/OMComponent.h>
 
 #include "MonoMethods.h"
+#include "ScriptSharedData.h"
 
 namespace fx::mono
 {
-class MonoScriptRuntime : public fx::OMClass<MonoScriptRuntime, IScriptRuntime, IScriptFileHandlingRuntime, IScriptTickRuntimeWithBookmarks,
+class MonoScriptRuntime : public fx::OMClass<MonoScriptRuntime, IScriptRuntime, IScriptFileHandlingRuntime, IScriptTickRuntime,
 	IScriptEventRuntime, IScriptRefRuntime, IScriptMemInfoRuntime, /*IScriptStackWalkingRuntime,*/ IScriptDebugRuntime, IScriptProfiler>
 {
 private:
@@ -35,22 +36,22 @@ class MonoScriptRuntime : public fx::OMClass<MonoScriptRuntime, IScriptRuntime,
 	IScriptHost* m_scriptHost;
 	IScriptHostWithResourceData* m_resourceHost;
 	IScriptHostWithManifest* m_manifestHost;
-	IScriptHostWithBookmarks* m_bookmarkHost;
 
 	fx::OMPtr<IScriptRuntimeHandler> m_handler;
 	fx::Resource* m_parentObject;
 	IDebugEventListener* m_debugListener;
 	std::unordered_map<std::string, int> m_scriptIds;
-	uint64_t m_scheduledTime = ~uint64_t(0);
+
+	ScriptSharedData m_sharedData;
 
 	// method targets
 	Method m_loadAssembly;
 
 	// method thunks, these are for calls that require performance
-	Thunk<uint32_t(uint64_t gameTime, bool profiling)> m_tick;
-	Thunk<uint32_t(MonoString* eventName, const char* argsSerialized, uint32_t serializedSize, MonoString* sourceId, uint64_t gameTime, bool profiling)> m_triggerEvent;
+	Thunk<void(uint64_t gameTime, bool profiling)> m_tick;
+	Thunk<void(MonoString* eventName, const char* argsSerialized, uint32_t serializedSize, MonoString* sourceId, uint64_t gameTime, bool profiling)> m_triggerEvent;
 
-	Thunk<uint32_t(int32_t refIndex, char* argsSerialized, uint32_t argsSize, char** retvalSerialized, uint32_t* retvalSize, uint64_t gameTime, bool profiling)> m_callRef;
+	Thunk<void(int32_t refIndex, char* argsSerialized, uint32_t argsSize, char** retvalSerialized, uint32_t* retvalSize, uint64_t gameTime, bool profiling)> m_callRef;
 	Thunk<void(int32_t refIndex, int32_t* newRefIdx)> m_duplicateRef = nullptr;
 	Thunk<void(int32_t refIndex)> m_removeRef = nullptr;
 
@@ -75,9 +76,7 @@ class MonoScriptRuntime : public fx::OMClass<MonoScriptRuntime, IScriptRuntime,
 
 	NS_DECL_ISCRIPTFILEHANDLINGRUNTIME;
 
-	NS_DECL_ISCRIPTTICKRUNTIMEWITHBOOKMARKS;
-
-	void ScheduleTick(uint64_t timeMilliseconds);
+	NS_DECL_ISCRIPTTICKRUNTIME;
 
 	NS_DECL_ISCRIPTEVENTRUNTIME;
 
@@ -97,13 +96,4 @@ inline MonoScriptRuntime::MonoScriptRuntime()
 	m_instanceId = rand();
 	m_name = "ScriptDomain_" + std::to_string(m_instanceId);
 }
-
-inline void MonoScriptRuntime::ScheduleTick(uint64_t timeMilliseconds)
-{
-	if (timeMilliseconds < m_scheduledTime)
-	{
-		m_scheduledTime = timeMilliseconds;
-		m_bookmarkHost->ScheduleBookmark(this, 0, timeMilliseconds * 1000); // positive values are expected to me microseconds and absolute
-	}
-}
 }

code/components/citizen-scripting-mono-v2/include/ScriptSharedData.h

@@ -0,0 +1,12 @@
+#pragma once
+
+#include <atomic>
+
+namespace fx::mono
+{
+struct ScriptSharedData
+{
+public:
+	std::atomic<uint64_t> m_scheduledTime = ~uint64_t(0);
+};
+}

code/components/citizen-scripting-mono-v2/src/MonoScriptRuntime.cpp

@@ -104,11 +104,6 @@ result_t MonoScriptRuntime::Create(IScriptHost* host)
 			fx::OMPtr<IScriptHostWithManifest> manifestPtr;
 			ptr.As(&manifestPtr);
 			m_manifestHost = manifestPtr.GetRef();
-
-			fx::OMPtr<IScriptHostWithBookmarks> bookmarkPtr;
-			ptr.As(&bookmarkPtr);
-			m_bookmarkHost = bookmarkPtr.GetRef();
-			m_bookmarkHost->CreateBookmarks(this);
 		}
 
 		char* resourceName = nullptr;
@@ -134,7 +129,7 @@ result_t MonoScriptRuntime::Create(IScriptHost* host)
 		auto* thisPtr = this;
 		MonoException* exc;
 		auto initialize = Method::Find(image, "CitizenFX.Core.ScriptInterface:Initialize");
-		initialize({ mono_string_new(m_appDomain, resourceName), &thisPtr, &m_instanceId }, &exc);
+		initialize({ mono_string_new(m_appDomain, resourceName), &thisPtr, &m_instanceId, &m_sharedData }, &exc);
 
 		mono_domain_set_internal(mono_get_root_domain()); // back to root for v1
 
@@ -172,16 +167,26 @@ result_t MonoScriptRuntime::Destroy()
 
 	m_appDomain = nullptr;
 	m_scriptHost = nullptr;
-	m_bookmarkHost->RemoveBookmarks(this);
-	m_bookmarkHost = nullptr;
 
 	mono_domain_set_internal(mono_get_root_domain()); // back to root for v1
 
 	return ReturnOrError(exc);
 }
 
-result_t MonoScriptRuntime::TickBookmarks(uint64_t* bookmarks, int32_t numBookmarks)
+result_t MonoScriptRuntime::Tick()
 {
+	// Tick-less: we don't pay for runtime entry and exit costs if there's nothing to do.
+	{
+		auto nextScheduledTime = m_sharedData.m_scheduledTime.load();
+		if (GetCurrentSchedulerTime() < nextScheduledTime)
+		{
+			return FX_S_OK;
+		}
+
+		// We can ignore the time between the load above and the store below as the runtime will set this value again if there's still work to do
+		m_sharedData.m_scheduledTime.store(~uint64_t(0));
+	}
+
 	m_handler->PushRuntime(static_cast<IScriptRuntime*>(this));
 	if (m_parentObject)
 		m_parentObject->OnActivate();
@@ -190,12 +195,8 @@ result_t MonoScriptRuntime::TickBookmarks(uint64_t* bookmarks, int32_t numBookma
 
 	MONO_BOUNDARY_START
 
-	// reset scheduled time, nextTick will set the next time
-	m_scheduledTime = ~uint64_t(0);
-
 	MonoException* exc;
-	uint64_t nextTick = m_tick(GetCurrentSchedulerTime(), IsProfiling(), &exc);
-	ScheduleTick(nextTick);
+	m_tick(GetCurrentSchedulerTime(), IsProfiling(), &exc);
 
 	MONO_BOUNDARY_END
 
@@ -216,12 +217,10 @@ result_t MonoScriptRuntime::TriggerEvent(char* eventName, char* argsSerialized,
 	MONO_BOUNDARY_START
 
 	MonoException* exc = nullptr;
-	uint64_t nextTick = m_triggerEvent(mono_string_new(m_appDomain, eventName),
+	m_triggerEvent(mono_string_new(m_appDomain, eventName),
 		argsSerialized, serializedSize, mono_string_new(m_appDomain, sourceId),
 		GetCurrentSchedulerTime(), IsProfiling(), &exc);
 
-	ScheduleTick(nextTick);
-
 	MONO_BOUNDARY_END
 
 	return ReturnOrError(exc);
@@ -327,9 +326,7 @@ result_t MonoScriptRuntime::LoadFile(char* scriptFile)
 	bool isProfiling = IsProfiling();
 
 	MonoException* exc = nullptr;
-	MonoObject* nextTickObject = m_loadAssembly({ mono_string_new(m_appDomain, scriptFile), &currentTime, &isProfiling }, &exc);
-	uint64_t nextTick = *reinterpret_cast<uint64_t*>(mono_object_unbox(nextTickObject));
-	ScheduleTick(nextTick);
+	m_loadAssembly({ mono_string_new(m_appDomain, scriptFile), &currentTime, &isProfiling }, &exc);
 
 	console::PrintWarning(_CFX_NAME_STRING(_CFX_COMPONENT_NAME),
 		"Assembly %s has been loaded into the mono rt2 runtime. This runtime is still in beta and shouldn't be used in production, "
@@ -348,8 +345,7 @@ result_t MonoScriptRuntime::CallRef(int32_t refIndex, char* argsSerialized, uint
 	MonoDomainScope scope(m_appDomain);
 
 	MonoException* exc = nullptr;
-	uint64_t nextTick = m_callRef(refIndex, argsSerialized, argsSize, retvalSerialized, retvalSize, GetCurrentSchedulerTime(), IsProfiling(), &exc);
-	ScheduleTick(nextTick);
+	m_callRef(refIndex, argsSerialized, argsSize, retvalSerialized, retvalSize, GetCurrentSchedulerTime(), IsProfiling(), &exc);
 
 	return ReturnOrError(exc);
 }