h1or4dme8/osu-lazer
1
0
Fork 0
mirror of https://github.com/ppy/osu.git synced 2024-12-15 01:52:55 +08:00
Code Issues Releases Wiki Activity

Remove probably redundant realmLock

Browse Source 
As far as I can tell all accesses are safe due to update thread
guarantees. The only weird one may be async writes during a
`BlockAllOperations`, but the `Compact` loop should handle this quite
amicably.
This commit is contained in:
Dean Herbert 2022-06-28 16:54:53 +09:00
parent 6203885040
commit e10ac45fd7
1 changed files with 62 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

147
osu.Game/Database/RealmAccess.cs
View File

@ -98,8 +98,6 @@ namespace osu.Game.Database
private static readonly GlobalStatistic<int> total_writes_async = GlobalStatistics.Get<int>(@"Realm", @"Writes (Async)"); private static readonly GlobalStatistic<int> total_writes_async = GlobalStatistics.Get<int>(@"Realm", @"Writes (Async)");
private readonly object realmLock = new object();
private Realm? updateRealm; private Realm? updateRealm;
/// <summary> /// <summary>
@ -122,24 +120,21 @@ namespace osu.Game.Database
if (!ThreadSafety.IsUpdateThread) if (!ThreadSafety.IsUpdateThread)
throw new InvalidOperationException(@$"Use {nameof(getRealmInstance)} when performing realm operations from a non-update thread"); throw new InvalidOperationException(@$"Use {nameof(getRealmInstance)} when performing realm operations from a non-update thread");
lock (realmLock) if (updateRealm == null)
{ {
if (updateRealm == null) updateRealm = getRealmInstance();
{ hasInitialisedOnce = true;
updateRealm = getRealmInstance();
hasInitialisedOnce = true;
Logger.Log(@$"Opened realm ""{updateRealm.Config.DatabasePath}"" at version {updateRealm.Config.SchemaVersion}"); Logger.Log(@$"Opened realm ""{updateRealm.Config.DatabasePath}"" at version {updateRealm.Config.SchemaVersion}");
// Resubscribe any subscriptions // Resubscribe any subscriptions
foreach (var action in customSubscriptionsResetMap.Keys) foreach (var action in customSubscriptionsResetMap.Keys)
registerSubscription(action); registerSubscription(action);
}
Debug.Assert(updateRealm != null);
return updateRealm;
} }
Debug.Assert(updateRealm != null);
return updateRealm;
} }
internal static bool CurrentThreadSubscriptionsAllowed => current_thread_subscriptions_allowed.Value; internal static bool CurrentThreadSubscriptionsAllowed => current_thread_subscriptions_allowed.Value;
@ -404,30 +399,27 @@ namespace osu.Game.Database
if (!ThreadSafety.IsUpdateThread) if (!ThreadSafety.IsUpdateThread)
throw new InvalidOperationException(@$"{nameof(WriteAsync)} must be called from the update thread."); throw new InvalidOperationException(@$"{nameof(WriteAsync)} must be called from the update thread.");
lock (realmLock) // CountdownEvent will fail if already at zero.
if (!pendingAsyncWrites.TryAddCount())
pendingAsyncWrites.Reset(1);
// Regardless of calling Realm.GetInstance or Realm.GetInstanceAsync, there is a blocking overhead on retrieval.
// Adding a forced Task.Run resolves this.
var writeTask = Task.Run(async () =>
{ {
// CountdownEvent will fail if already at zero. total_writes_async.Value++;
if (!pendingAsyncWrites.TryAddCount())
pendingAsyncWrites.Reset(1);
// Regardless of calling Realm.GetInstance or Realm.GetInstanceAsync, there is a blocking overhead on retrieval. // Not attempting to use Realm.GetInstanceAsync as there's seemingly no benefit to us (for now) and it adds complexity due to locking
// Adding a forced Task.Run resolves this. // concerns in getRealmInstance(). On a quick check, it looks to be more suited to cases where realm is connecting to an online sync
var writeTask = Task.Run(async () => // server, which we don't use. May want to report upstream or revisit in the future.
{ using (var realm = getRealmInstance())
total_writes_async.Value++; // ReSharper disable once AccessToDisposedClosure (WriteAsync should be marked as [InstantHandle]).
await realm.WriteAsync(() => action(realm));
// Not attempting to use Realm.GetInstanceAsync as there's seemingly no benefit to us (for now) and it adds complexity due to locking pendingAsyncWrites.Signal();
// concerns in getRealmInstance(). On a quick check, it looks to be more suited to cases where realm is connecting to an online sync });
// server, which we don't use. May want to report upstream or revisit in the future.
using (var realm = getRealmInstance())
// ReSharper disable once AccessToDisposedClosure (WriteAsync should be marked as [InstantHandle]).
await realm.WriteAsync(() => action(realm));
pendingAsyncWrites.Signal(); return writeTask;
});
return writeTask;
}
} }
/// <summary> /// <summary>
@ -452,14 +444,11 @@ namespace osu.Game.Database
public IDisposable RegisterForNotifications<T>(Func<Realm, IQueryable<T>> query, NotificationCallbackDelegate<T> callback) public IDisposable RegisterForNotifications<T>(Func<Realm, IQueryable<T>> query, NotificationCallbackDelegate<T> callback)
where T : RealmObjectBase where T : RealmObjectBase
{ {
lock (realmLock) Func<Realm, IDisposable?> action = realm => query(realm).QueryAsyncWithNotifications(callback);
{
Func<Realm, IDisposable?> action = realm => query(realm).QueryAsyncWithNotifications(callback);
// Store an action which is used when blocking to ensure consumers don't use results of a stale changeset firing. // Store an action which is used when blocking to ensure consumers don't use results of a stale changeset firing.
notificationsResetMap.Add(action, () => callback(new EmptyRealmSet<T>(), null, null)); notificationsResetMap.Add(action, () => callback(new EmptyRealmSet<T>(), null, null));
return RegisterCustomSubscription(action); return RegisterCustomSubscription(action);
}
} }
/// <summary> /// <summary>
@ -550,15 +539,12 @@ namespace osu.Game.Database
void unsubscribe() void unsubscribe()
{ {
lock (realmLock) if (customSubscriptionsResetMap.TryGetValue(action, out var unsubscriptionAction))
{ {
if (customSubscriptionsResetMap.TryGetValue(action, out var unsubscriptionAction)) unsubscriptionAction?.Dispose();
{ customSubscriptionsResetMap.Remove(action);
unsubscriptionAction?.Dispose(); notificationsResetMap.Remove(action);
customSubscriptionsResetMap.Remove(action); total_subscriptions.Value--;
notificationsResetMap.Remove(action);
total_subscriptions.Value--;
}
} }
} }
}); });
@ -568,19 +554,16 @@ namespace osu.Game.Database
{ {
Debug.Assert(ThreadSafety.IsUpdateThread); Debug.Assert(ThreadSafety.IsUpdateThread);
lock (realmLock) // Retrieve realm instance outside of flag update to ensure that the instance is retrieved,
{ // as attempting to access it inside the subscription if it's not constructed would lead to
// Retrieve realm instance outside of flag update to ensure that the instance is retrieved, // cyclic invocations of the subscription callback.
// as attempting to access it inside the subscription if it's not constructed would lead to var realm = Realm;
// cyclic invocations of the subscription callback.
var realm = Realm;
Debug.Assert(!customSubscriptionsResetMap.TryGetValue(action, out var found) || found == null); Debug.Assert(!customSubscriptionsResetMap.TryGetValue(action, out var found) || found == null);
current_thread_subscriptions_allowed.Value = true; current_thread_subscriptions_allowed.Value = true;
customSubscriptionsResetMap[action] = action(realm); customSubscriptionsResetMap[action] = action(realm);
current_thread_subscriptions_allowed.Value = false; current_thread_subscriptions_allowed.Value = false;
}
} }
private Realm getRealmInstance() private Realm getRealmInstance()
@ -831,31 +814,28 @@ namespace osu.Game.Database
{ {
realmRetrievalLock.Wait(); realmRetrievalLock.Wait();
lock (realmLock) if (hasInitialisedOnce)
{ {
if (hasInitialisedOnce) if (!ThreadSafety.IsUpdateThread)
throw new InvalidOperationException(@$"{nameof(BlockAllOperations)} must be called from the update thread.");
syncContext = SynchronizationContext.Current;
// Before disposing the update context, clean up all subscriptions.
// Note that in the case of realm notification subscriptions, this is not really required (they will be cleaned up by disposal).
// In the case of custom subscriptions, we want them to fire before the update realm is disposed in case they do any follow-up work.
foreach (var action in customSubscriptionsResetMap.ToArray())
{ {
if (!ThreadSafety.IsUpdateThread) action.Value?.Dispose();
throw new InvalidOperationException(@$"{nameof(BlockAllOperations)} must be called from the update thread."); customSubscriptionsResetMap[action.Key] = null;
syncContext = SynchronizationContext.Current;
// Before disposing the update context, clean up all subscriptions.
// Note that in the case of realm notification subscriptions, this is not really required (they will be cleaned up by disposal).
// In the case of custom subscriptions, we want them to fire before the update realm is disposed in case they do any follow-up work.
foreach (var action in customSubscriptionsResetMap.ToArray())
{
action.Value?.Dispose();
customSubscriptionsResetMap[action.Key] = null;
}
updateRealm?.Dispose();
updateRealm = null;
} }
Logger.Log(@"Blocking realm operations.", LoggingTarget.Database); updateRealm?.Dispose();
updateRealm = null;
} }
Logger.Log(@"Blocking realm operations.", LoggingTarget.Database);
const int sleep_length = 200; const int sleep_length = 200;
int timeout = 5000; int timeout = 5000;
@ -933,10 +913,7 @@ namespace osu.Game.Database
if (!pendingAsyncWrites.Wait(10000)) if (!pendingAsyncWrites.Wait(10000))
Logger.Log("Realm took too long waiting on pending async writes", level: LogLevel.Error); Logger.Log("Realm took too long waiting on pending async writes", level: LogLevel.Error);
lock (realmLock) updateRealm?.Dispose();
{
updateRealm?.Dispose();
}
if (!isDisposed) if (!isDisposed)
{ {