From 35ea362cb2a13105bcc6f7a921985b9824693681 Mon Sep 17 00:00:00 2001 From: Alexander Zaytsev Date: Thu, 11 Apr 2019 22:16:25 +1200 Subject: [PATCH 1/5] Prevent calling UpdateTimestampsCache if query spaces are not changed Fixes #2115 --- src/NHibernate/Cache/StandardQueryCache.cs | 2 +- src/NHibernate/Cache/UpdateTimestampsCache.cs | 8 +------- src/NHibernate/Engine/ActionQueue.cs | 4 ++-- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/NHibernate/Cache/StandardQueryCache.cs b/src/NHibernate/Cache/StandardQueryCache.cs index d63134d3ec8..28236beb549 100644 --- a/src/NHibernate/Cache/StandardQueryCache.cs +++ b/src/NHibernate/Cache/StandardQueryCache.cs @@ -551,7 +551,7 @@ private static ICacheAssembler[] GuessTypes(IList cacheable) protected virtual bool IsUpToDate(ISet spaces, long timestamp) { - return _updateTimestampsCache.IsUpToDate(spaces, timestamp); + return spaces.Count == 0 || _updateTimestampsCache.IsUpToDate(spaces, timestamp); } } } diff --git a/src/NHibernate/Cache/UpdateTimestampsCache.cs b/src/NHibernate/Cache/UpdateTimestampsCache.cs index 21f37cf8d25..c7270daf3a5 100644 --- a/src/NHibernate/Cache/UpdateTimestampsCache.cs +++ b/src/NHibernate/Cache/UpdateTimestampsCache.cs @@ -103,13 +103,7 @@ public virtual bool IsUpToDate(ISet spaces, long timestamp /* H2.1 has L if (spaces.Count == 0) return true; - var keys = new object[spaces.Count]; - var index = 0; - foreach (var space in spaces) - { - keys[index++] = space; - } - var lastUpdates = _updateTimestamps.GetMany(keys); + var lastUpdates = _updateTimestamps.GetMany(spaces.ToArray()); return lastUpdates.All(lastUpdate => !IsOutdated(lastUpdate as long?, timestamp)); } diff --git a/src/NHibernate/Engine/ActionQueue.cs b/src/NHibernate/Engine/ActionQueue.cs index 5c33537779c..99c1fee90e8 100644 --- a/src/NHibernate/Engine/ActionQueue.cs +++ b/src/NHibernate/Engine/ActionQueue.cs @@ -173,7 +173,7 @@ private void ExecuteActions(IList list) private void PreInvalidateCaches() { - if (session.Factory.Settings.IsQueryCacheEnabled) + if (session.Factory.Settings.IsQueryCacheEnabled && executedSpaces.Count > 0) { session.Factory.UpdateTimestampsCache.PreInvalidate(executedSpaces); } @@ -295,7 +295,7 @@ public void AfterTransactionCompletion(bool success) private void InvalidateCaches() { - if (session.Factory.Settings.IsQueryCacheEnabled) + if (session.Factory.Settings.IsQueryCacheEnabled && executedSpaces.Count > 0) { session.Factory.UpdateTimestampsCache.Invalidate(executedSpaces); } From 46a566e322fb52dc7a7ff68f800fa34c1d94ee17 Mon Sep 17 00:00:00 2001 From: Alexander Zaytsev Date: Fri, 12 Apr 2019 12:20:11 +1200 Subject: [PATCH 2/5] fixup! Prevent calling UpdateTimestampsCache if query spaces are not changed --- src/NHibernate/Async/Cache/StandardQueryCache.cs | 9 +++------ src/NHibernate/Async/Cache/UpdateTimestampsCache.cs | 8 +------- src/NHibernate/Async/Engine/ActionQueue.cs | 4 ++-- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/NHibernate/Async/Cache/StandardQueryCache.cs b/src/NHibernate/Async/Cache/StandardQueryCache.cs index ae75c393656..1063dafb67d 100644 --- a/src/NHibernate/Async/Cache/StandardQueryCache.cs +++ b/src/NHibernate/Async/Cache/StandardQueryCache.cs @@ -464,13 +464,10 @@ private async Task GetResultFromCacheableAsync( } } - protected virtual Task IsUpToDateAsync(ISet spaces, long timestamp, CancellationToken cancellationToken) + protected virtual async Task IsUpToDateAsync(ISet spaces, long timestamp, CancellationToken cancellationToken) { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } - return _updateTimestampsCache.IsUpToDateAsync(spaces, timestamp, cancellationToken); + cancellationToken.ThrowIfCancellationRequested(); + return spaces.Count == 0 || await (_updateTimestampsCache.IsUpToDateAsync(spaces, timestamp, cancellationToken)).ConfigureAwait(false); } } } diff --git a/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs b/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs index e5ba6ce0e45..98d6d275326 100644 --- a/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs +++ b/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs @@ -139,13 +139,7 @@ public virtual async Task IsUpToDateAsync(ISet spaces, long timest if (spaces.Count == 0) return true; - var keys = new object[spaces.Count]; - var index = 0; - foreach (var space in spaces) - { - keys[index++] = space; - } - var lastUpdates = await (_updateTimestamps.GetManyAsync(keys, cancellationToken)).ConfigureAwait(false); + var lastUpdates = await (_updateTimestamps.GetManyAsync(spaces.ToArray(), cancellationToken)).ConfigureAwait(false); return lastUpdates.All(lastUpdate => !IsOutdated(lastUpdate as long?, timestamp)); } } diff --git a/src/NHibernate/Async/Engine/ActionQueue.cs b/src/NHibernate/Async/Engine/ActionQueue.cs index 9a405ab0087..565722ef477 100644 --- a/src/NHibernate/Async/Engine/ActionQueue.cs +++ b/src/NHibernate/Async/Engine/ActionQueue.cs @@ -46,7 +46,7 @@ private Task PreInvalidateCachesAsync(CancellationToken cancellationToken) { return Task.FromCanceled(cancellationToken); } - if (session.Factory.Settings.IsQueryCacheEnabled) + if (session.Factory.Settings.IsQueryCacheEnabled && executedSpaces.Count > 0) { return session.Factory.UpdateTimestampsCache.PreInvalidateAsync(executedSpaces, cancellationToken); } @@ -166,7 +166,7 @@ public async Task AfterTransactionCompletionAsync(bool success, CancellationToke private async Task InvalidateCachesAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - if (session.Factory.Settings.IsQueryCacheEnabled) + if (session.Factory.Settings.IsQueryCacheEnabled && executedSpaces.Count > 0) { await (session.Factory.UpdateTimestampsCache.InvalidateAsync(executedSpaces, cancellationToken)).ConfigureAwait(false); } From 58692a26bdc2794575547fc8f72ac2a25a78f97e Mon Sep 17 00:00:00 2001 From: Alexander Zaytsev Date: Fri, 12 Apr 2019 12:32:17 +1200 Subject: [PATCH 3/5] Use ReaderWriterLockSlim instead of synchronized methods --- .../Async/Cache/UpdateTimestampsCache.cs | 119 ++++++++-------- src/NHibernate/Cache/UpdateTimestampsCache.cs | 127 +++++++++++------- 2 files changed, 139 insertions(+), 107 deletions(-) diff --git a/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs b/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs index 98d6d275326..fce5a825037 100644 --- a/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs +++ b/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs @@ -11,21 +11,15 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Runtime.CompilerServices; - +using System.Threading; using NHibernate.Cfg; using NHibernate.Util; namespace NHibernate.Cache { using System.Threading.Tasks; - using System.Threading; public partial class UpdateTimestampsCache { - private readonly NHibernate.Util.AsyncLock _preInvalidate = new NHibernate.Util.AsyncLock(); - private readonly NHibernate.Util.AsyncLock _invalidate = new NHibernate.Util.AsyncLock(); - private readonly NHibernate.Util.AsyncLock _isUpToDate = new NHibernate.Util.AsyncLock(); - private readonly NHibernate.Util.AsyncLock _areUpToDate = new NHibernate.Util.AsyncLock(); public virtual Task ClearAsync(CancellationToken cancellationToken) { @@ -55,20 +49,24 @@ public Task PreInvalidateAsync(object[] spaces, CancellationToken cancellationTo } } - [MethodImpl()] public virtual async Task PreInvalidateAsync(IReadOnlyCollection spaces, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - using (await _preInvalidate.LockAsync()) + if (spaces.Count == 0) + return; + + Lock.EnterUpgradeableReadLock(); + try { //TODO: to handle concurrent writes correctly, this should return a Lock to the client - var ts = _updateTimestamps.NextTimestamp() + _updateTimestamps.Timeout; - await (SetSpacesTimestampAsync(spaces, ts, cancellationToken)).ConfigureAwait(false); - + var timestamp = _updateTimestamps.NextTimestamp() + _updateTimestamps.Timeout; + await (SetSpacesTimestampAsync(spaces, timestamp, cancellationToken)).ConfigureAwait(false); //TODO: return new Lock(ts); } - - //TODO: return new Lock(ts); + finally + { + Lock.ExitUpgradeableReadLock(); + } } //Since v5.1 @@ -90,11 +88,14 @@ public Task InvalidateAsync(object[] spaces, CancellationToken cancellationToken } } - [MethodImpl()] public virtual async Task InvalidateAsync(IReadOnlyCollection spaces, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - using (await _invalidate.LockAsync()) + if (spaces.Count == 0) + return; + + Lock.EnterUpgradeableReadLock(); + try { //TODO: to handle concurrent writes correctly, the client should pass in a Lock long ts = _updateTimestamps.NextTimestamp(); @@ -103,70 +104,65 @@ public virtual async Task InvalidateAsync(IReadOnlyCollection spaces, Ca log.Debug("Invalidating spaces [{0}]", StringHelper.CollectionToString(spaces)); await (SetSpacesTimestampAsync(spaces, ts, cancellationToken)).ConfigureAwait(false); } + finally + { + Lock.ExitUpgradeableReadLock(); + } } - private Task SetSpacesTimestampAsync(IReadOnlyCollection spaces, long ts, CancellationToken cancellationToken) + private async Task SetSpacesTimestampAsync(IReadOnlyCollection spaces, long ts, CancellationToken cancellationToken) { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } + cancellationToken.ThrowIfCancellationRequested(); + Lock.EnterWriteLock(); try { - if (spaces.Count == 0) - return Task.CompletedTask; - - var timestamps = new object[spaces.Count]; - for (var i = 0; i < timestamps.Length; i++) - { - timestamps[i] = ts; - } - - return _updateTimestamps.PutManyAsync(spaces.ToArray(), timestamps, cancellationToken); + var timestamps = ArrayHelper.Fill(ts, spaces.Count); + await (_updateTimestamps.PutManyAsync(spaces.ToArray(), timestamps, cancellationToken)).ConfigureAwait(false); } - catch (Exception ex) + finally { - return Task.FromException(ex); + Lock.ExitWriteLock(); } } - [MethodImpl()] public virtual async Task IsUpToDateAsync(ISet spaces, long timestamp /* H2.1 has Long here */, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - using (await _isUpToDate.LockAsync()) - { - if (spaces.Count == 0) - return true; + if (spaces.Count == 0) + return true; - var lastUpdates = await (_updateTimestamps.GetManyAsync(spaces.ToArray(), cancellationToken)).ConfigureAwait(false); + Lock.EnterReadLock(); + try + { + var lastUpdates = await (_updateTimestamps.GetManyAsync(spaces.ToArray(), cancellationToken)).ConfigureAwait(false); return lastUpdates.All(lastUpdate => !IsOutdated(lastUpdate as long?, timestamp)); } + finally + { + Lock.ExitReadLock(); + } } - [MethodImpl()] public virtual async Task AreUpToDateAsync(ISet[] spaces, long[] timestamps, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - using (await _areUpToDate.LockAsync()) - { - var results = new bool[spaces.Length]; - var allSpaces = new HashSet(); - foreach (var sp in spaces) - { - allSpaces.UnionWith(sp); - } + if (spaces.Length == 0) + return Array.Empty(); - if (allSpaces.Count == 0) - { - for (var i = 0; i < spaces.Length; i++) - { - results[i] = true; - } + var allSpaces = new HashSet(); + foreach (var sp in spaces) + { + allSpaces.UnionWith(sp); + } - return results; - } + if (allSpaces.Count == 0) + { + return ArrayHelper.Fill(true, spaces.Length); + } + Lock.EnterReadLock(); + try + { var keys = new object[allSpaces.Count]; var index = 0; foreach (var space in allSpaces) @@ -176,10 +172,11 @@ public virtual async Task AreUpToDateAsync(ISet[] spaces, long[] index = 0; var lastUpdatesBySpace = - (await (_updateTimestamps - .GetManyAsync(keys, cancellationToken)).ConfigureAwait(false)) - .ToDictionary(u => keys[index++], u => u as long?); + (await (_updateTimestamps + .GetManyAsync(keys, cancellationToken)).ConfigureAwait(false)) + .ToDictionary(u => keys[index++], u => u as long?); + var results = new bool[spaces.Length]; for (var i = 0; i < spaces.Length; i++) { var timestamp = timestamps[i]; @@ -188,6 +185,10 @@ public virtual async Task AreUpToDateAsync(ISet[] spaces, long[] return results; } + finally + { + Lock.ExitReadLock(); + } } } } diff --git a/src/NHibernate/Cache/UpdateTimestampsCache.cs b/src/NHibernate/Cache/UpdateTimestampsCache.cs index c7270daf3a5..d2777dcf55c 100644 --- a/src/NHibernate/Cache/UpdateTimestampsCache.cs +++ b/src/NHibernate/Cache/UpdateTimestampsCache.cs @@ -1,8 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Runtime.CompilerServices; - +using System.Threading; using NHibernate.Cfg; using NHibernate.Util; @@ -17,6 +16,7 @@ namespace NHibernate.Cache /// public partial class UpdateTimestampsCache { + private static readonly ReaderWriterLockSlim Lock = new ReaderWriterLockSlim(); private static readonly INHibernateLogger log = NHibernateLogger.For(typeof(UpdateTimestampsCache)); private readonly CacheBase _updateTimestamps; @@ -54,14 +54,23 @@ public void PreInvalidate(object[] spaces) PreInvalidate(spaces.OfType().ToList()); } - [MethodImpl(MethodImplOptions.Synchronized)] public virtual void PreInvalidate(IReadOnlyCollection spaces) { - //TODO: to handle concurrent writes correctly, this should return a Lock to the client - var ts = _updateTimestamps.NextTimestamp() + _updateTimestamps.Timeout; - SetSpacesTimestamp(spaces, ts); + if (spaces.Count == 0) + return; - //TODO: return new Lock(ts); + Lock.EnterUpgradeableReadLock(); + try + { + //TODO: to handle concurrent writes correctly, this should return a Lock to the client + var timestamp = _updateTimestamps.NextTimestamp() + _updateTimestamps.Timeout; + SetSpacesTimestamp(spaces, timestamp); + //TODO: return new Lock(ts); + } + finally + { + Lock.ExitUpgradeableReadLock(); + } } //Since v5.1 @@ -72,45 +81,63 @@ public void Invalidate(object[] spaces) Invalidate(spaces.OfType().ToList()); } - [MethodImpl(MethodImplOptions.Synchronized)] public virtual void Invalidate(IReadOnlyCollection spaces) - { - //TODO: to handle concurrent writes correctly, the client should pass in a Lock - long ts = _updateTimestamps.NextTimestamp(); - //TODO: if lock.getTimestamp().equals(ts) - if (log.IsDebugEnabled()) - log.Debug("Invalidating spaces [{0}]", StringHelper.CollectionToString(spaces)); - SetSpacesTimestamp(spaces, ts); - } - - private void SetSpacesTimestamp(IReadOnlyCollection spaces, long ts) { if (spaces.Count == 0) return; - var timestamps = new object[spaces.Count]; - for (var i = 0; i < timestamps.Length; i++) + Lock.EnterUpgradeableReadLock(); + try + { + //TODO: to handle concurrent writes correctly, the client should pass in a Lock + long ts = _updateTimestamps.NextTimestamp(); + //TODO: if lock.getTimestamp().equals(ts) + if (log.IsDebugEnabled()) + log.Debug("Invalidating spaces [{0}]", StringHelper.CollectionToString(spaces)); + SetSpacesTimestamp(spaces, ts); + } + finally { - timestamps[i] = ts; + Lock.ExitUpgradeableReadLock(); } + } - _updateTimestamps.PutMany(spaces.ToArray(), timestamps); + private void SetSpacesTimestamp(IReadOnlyCollection spaces, long ts) + { + Lock.EnterWriteLock(); + try + { + var timestamps = ArrayHelper.Fill(ts, spaces.Count); + _updateTimestamps.PutMany(spaces.ToArray(), timestamps); + } + finally + { + Lock.ExitWriteLock(); + } } - [MethodImpl(MethodImplOptions.Synchronized)] public virtual bool IsUpToDate(ISet spaces, long timestamp /* H2.1 has Long here */) { if (spaces.Count == 0) return true; - var lastUpdates = _updateTimestamps.GetMany(spaces.ToArray()); - return lastUpdates.All(lastUpdate => !IsOutdated(lastUpdate as long?, timestamp)); + Lock.EnterReadLock(); + try + { + var lastUpdates = _updateTimestamps.GetMany(spaces.ToArray()); + return lastUpdates.All(lastUpdate => !IsOutdated(lastUpdate as long?, timestamp)); + } + finally + { + Lock.ExitReadLock(); + } } - [MethodImpl(MethodImplOptions.Synchronized)] public virtual bool[] AreUpToDate(ISet[] spaces, long[] timestamps) { - var results = new bool[spaces.Length]; + if (spaces.Length == 0) + return Array.Empty(); + var allSpaces = new HashSet(); foreach (var sp in spaces) { @@ -119,34 +146,38 @@ public virtual bool[] AreUpToDate(ISet[] spaces, long[] timestamps) if (allSpaces.Count == 0) { + return ArrayHelper.Fill(true, spaces.Length); + } + + Lock.EnterReadLock(); + try + { + var keys = new object[allSpaces.Count]; + var index = 0; + foreach (var space in allSpaces) + { + keys[index++] = space; + } + + index = 0; + var lastUpdatesBySpace = + _updateTimestamps + .GetMany(keys) + .ToDictionary(u => keys[index++], u => u as long?); + + var results = new bool[spaces.Length]; for (var i = 0; i < spaces.Length; i++) { - results[i] = true; + var timestamp = timestamps[i]; + results[i] = spaces[i].All(space => !IsOutdated(lastUpdatesBySpace[space], timestamp)); } return results; } - - var keys = new object[allSpaces.Count]; - var index = 0; - foreach (var space in allSpaces) + finally { - keys[index++] = space; + Lock.ExitReadLock(); } - - index = 0; - var lastUpdatesBySpace = - _updateTimestamps - .GetMany(keys) - .ToDictionary(u => keys[index++], u => u as long?); - - for (var i = 0; i < spaces.Length; i++) - { - var timestamp = timestamps[i]; - results[i] = spaces[i].All(space => !IsOutdated(lastUpdatesBySpace[space], timestamp)); - } - - return results; } // Since v5.3 @@ -157,7 +188,7 @@ public virtual void Destroy() // not the responsibility of this class. } - private bool IsOutdated(long? lastUpdate, long timestamp) + private static bool IsOutdated(long? lastUpdate, long timestamp) { if (!lastUpdate.HasValue) { From 830ed6428a68d7507acbe9e6ff812d8ad31a9acb Mon Sep 17 00:00:00 2001 From: Alexander Zaytsev Date: Wed, 1 May 2019 15:57:44 +1200 Subject: [PATCH 4/5] Revert "Use ReaderWriterLockSlim instead of synchronized methods" This reverts commit 58692a26bdc2794575547fc8f72ac2a25a78f97e. --- .../Async/Cache/UpdateTimestampsCache.cs | 119 ++++++++-------- src/NHibernate/Cache/UpdateTimestampsCache.cs | 127 +++++++----------- 2 files changed, 107 insertions(+), 139 deletions(-) diff --git a/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs b/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs index fce5a825037..98d6d275326 100644 --- a/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs +++ b/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs @@ -11,15 +11,21 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading; +using System.Runtime.CompilerServices; + using NHibernate.Cfg; using NHibernate.Util; namespace NHibernate.Cache { using System.Threading.Tasks; + using System.Threading; public partial class UpdateTimestampsCache { + private readonly NHibernate.Util.AsyncLock _preInvalidate = new NHibernate.Util.AsyncLock(); + private readonly NHibernate.Util.AsyncLock _invalidate = new NHibernate.Util.AsyncLock(); + private readonly NHibernate.Util.AsyncLock _isUpToDate = new NHibernate.Util.AsyncLock(); + private readonly NHibernate.Util.AsyncLock _areUpToDate = new NHibernate.Util.AsyncLock(); public virtual Task ClearAsync(CancellationToken cancellationToken) { @@ -49,24 +55,20 @@ public Task PreInvalidateAsync(object[] spaces, CancellationToken cancellationTo } } + [MethodImpl()] public virtual async Task PreInvalidateAsync(IReadOnlyCollection spaces, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - if (spaces.Count == 0) - return; - - Lock.EnterUpgradeableReadLock(); - try + using (await _preInvalidate.LockAsync()) { //TODO: to handle concurrent writes correctly, this should return a Lock to the client - var timestamp = _updateTimestamps.NextTimestamp() + _updateTimestamps.Timeout; - await (SetSpacesTimestampAsync(spaces, timestamp, cancellationToken)).ConfigureAwait(false); + var ts = _updateTimestamps.NextTimestamp() + _updateTimestamps.Timeout; + await (SetSpacesTimestampAsync(spaces, ts, cancellationToken)).ConfigureAwait(false); + //TODO: return new Lock(ts); } - finally - { - Lock.ExitUpgradeableReadLock(); - } + + //TODO: return new Lock(ts); } //Since v5.1 @@ -88,14 +90,11 @@ public Task InvalidateAsync(object[] spaces, CancellationToken cancellationToken } } + [MethodImpl()] public virtual async Task InvalidateAsync(IReadOnlyCollection spaces, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - if (spaces.Count == 0) - return; - - Lock.EnterUpgradeableReadLock(); - try + using (await _invalidate.LockAsync()) { //TODO: to handle concurrent writes correctly, the client should pass in a Lock long ts = _updateTimestamps.NextTimestamp(); @@ -104,65 +103,70 @@ public virtual async Task InvalidateAsync(IReadOnlyCollection spaces, Ca log.Debug("Invalidating spaces [{0}]", StringHelper.CollectionToString(spaces)); await (SetSpacesTimestampAsync(spaces, ts, cancellationToken)).ConfigureAwait(false); } - finally - { - Lock.ExitUpgradeableReadLock(); - } } - private async Task SetSpacesTimestampAsync(IReadOnlyCollection spaces, long ts, CancellationToken cancellationToken) + private Task SetSpacesTimestampAsync(IReadOnlyCollection spaces, long ts, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); - Lock.EnterWriteLock(); + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); + } try { - var timestamps = ArrayHelper.Fill(ts, spaces.Count); - await (_updateTimestamps.PutManyAsync(spaces.ToArray(), timestamps, cancellationToken)).ConfigureAwait(false); + if (spaces.Count == 0) + return Task.CompletedTask; + + var timestamps = new object[spaces.Count]; + for (var i = 0; i < timestamps.Length; i++) + { + timestamps[i] = ts; + } + + return _updateTimestamps.PutManyAsync(spaces.ToArray(), timestamps, cancellationToken); } - finally + catch (Exception ex) { - Lock.ExitWriteLock(); + return Task.FromException(ex); } } + [MethodImpl()] public virtual async Task IsUpToDateAsync(ISet spaces, long timestamp /* H2.1 has Long here */, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - if (spaces.Count == 0) - return true; - - Lock.EnterReadLock(); - try + using (await _isUpToDate.LockAsync()) { - var lastUpdates = await (_updateTimestamps.GetManyAsync(spaces.ToArray(), cancellationToken)).ConfigureAwait(false); + if (spaces.Count == 0) + return true; + + var lastUpdates = await (_updateTimestamps.GetManyAsync(spaces.ToArray(), cancellationToken)).ConfigureAwait(false); return lastUpdates.All(lastUpdate => !IsOutdated(lastUpdate as long?, timestamp)); } - finally - { - Lock.ExitReadLock(); - } } + [MethodImpl()] public virtual async Task AreUpToDateAsync(ISet[] spaces, long[] timestamps, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - if (spaces.Length == 0) - return Array.Empty(); - - var allSpaces = new HashSet(); - foreach (var sp in spaces) + using (await _areUpToDate.LockAsync()) { - allSpaces.UnionWith(sp); - } + var results = new bool[spaces.Length]; + var allSpaces = new HashSet(); + foreach (var sp in spaces) + { + allSpaces.UnionWith(sp); + } - if (allSpaces.Count == 0) - { - return ArrayHelper.Fill(true, spaces.Length); - } + if (allSpaces.Count == 0) + { + for (var i = 0; i < spaces.Length; i++) + { + results[i] = true; + } + + return results; + } - Lock.EnterReadLock(); - try - { var keys = new object[allSpaces.Count]; var index = 0; foreach (var space in allSpaces) @@ -172,11 +176,10 @@ public virtual async Task AreUpToDateAsync(ISet[] spaces, long[] index = 0; var lastUpdatesBySpace = - (await (_updateTimestamps - .GetManyAsync(keys, cancellationToken)).ConfigureAwait(false)) - .ToDictionary(u => keys[index++], u => u as long?); + (await (_updateTimestamps + .GetManyAsync(keys, cancellationToken)).ConfigureAwait(false)) + .ToDictionary(u => keys[index++], u => u as long?); - var results = new bool[spaces.Length]; for (var i = 0; i < spaces.Length; i++) { var timestamp = timestamps[i]; @@ -185,10 +188,6 @@ public virtual async Task AreUpToDateAsync(ISet[] spaces, long[] return results; } - finally - { - Lock.ExitReadLock(); - } } } } diff --git a/src/NHibernate/Cache/UpdateTimestampsCache.cs b/src/NHibernate/Cache/UpdateTimestampsCache.cs index d2777dcf55c..c7270daf3a5 100644 --- a/src/NHibernate/Cache/UpdateTimestampsCache.cs +++ b/src/NHibernate/Cache/UpdateTimestampsCache.cs @@ -1,7 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading; +using System.Runtime.CompilerServices; + using NHibernate.Cfg; using NHibernate.Util; @@ -16,7 +17,6 @@ namespace NHibernate.Cache /// public partial class UpdateTimestampsCache { - private static readonly ReaderWriterLockSlim Lock = new ReaderWriterLockSlim(); private static readonly INHibernateLogger log = NHibernateLogger.For(typeof(UpdateTimestampsCache)); private readonly CacheBase _updateTimestamps; @@ -54,23 +54,14 @@ public void PreInvalidate(object[] spaces) PreInvalidate(spaces.OfType().ToList()); } + [MethodImpl(MethodImplOptions.Synchronized)] public virtual void PreInvalidate(IReadOnlyCollection spaces) { - if (spaces.Count == 0) - return; + //TODO: to handle concurrent writes correctly, this should return a Lock to the client + var ts = _updateTimestamps.NextTimestamp() + _updateTimestamps.Timeout; + SetSpacesTimestamp(spaces, ts); - Lock.EnterUpgradeableReadLock(); - try - { - //TODO: to handle concurrent writes correctly, this should return a Lock to the client - var timestamp = _updateTimestamps.NextTimestamp() + _updateTimestamps.Timeout; - SetSpacesTimestamp(spaces, timestamp); - //TODO: return new Lock(ts); - } - finally - { - Lock.ExitUpgradeableReadLock(); - } + //TODO: return new Lock(ts); } //Since v5.1 @@ -81,63 +72,45 @@ public void Invalidate(object[] spaces) Invalidate(spaces.OfType().ToList()); } + [MethodImpl(MethodImplOptions.Synchronized)] public virtual void Invalidate(IReadOnlyCollection spaces) { - if (spaces.Count == 0) - return; - - Lock.EnterUpgradeableReadLock(); - try - { - //TODO: to handle concurrent writes correctly, the client should pass in a Lock - long ts = _updateTimestamps.NextTimestamp(); - //TODO: if lock.getTimestamp().equals(ts) - if (log.IsDebugEnabled()) - log.Debug("Invalidating spaces [{0}]", StringHelper.CollectionToString(spaces)); - SetSpacesTimestamp(spaces, ts); - } - finally - { - Lock.ExitUpgradeableReadLock(); - } + //TODO: to handle concurrent writes correctly, the client should pass in a Lock + long ts = _updateTimestamps.NextTimestamp(); + //TODO: if lock.getTimestamp().equals(ts) + if (log.IsDebugEnabled()) + log.Debug("Invalidating spaces [{0}]", StringHelper.CollectionToString(spaces)); + SetSpacesTimestamp(spaces, ts); } private void SetSpacesTimestamp(IReadOnlyCollection spaces, long ts) { - Lock.EnterWriteLock(); - try - { - var timestamps = ArrayHelper.Fill(ts, spaces.Count); - _updateTimestamps.PutMany(spaces.ToArray(), timestamps); - } - finally + if (spaces.Count == 0) + return; + + var timestamps = new object[spaces.Count]; + for (var i = 0; i < timestamps.Length; i++) { - Lock.ExitWriteLock(); + timestamps[i] = ts; } + + _updateTimestamps.PutMany(spaces.ToArray(), timestamps); } + [MethodImpl(MethodImplOptions.Synchronized)] public virtual bool IsUpToDate(ISet spaces, long timestamp /* H2.1 has Long here */) { if (spaces.Count == 0) return true; - Lock.EnterReadLock(); - try - { - var lastUpdates = _updateTimestamps.GetMany(spaces.ToArray()); - return lastUpdates.All(lastUpdate => !IsOutdated(lastUpdate as long?, timestamp)); - } - finally - { - Lock.ExitReadLock(); - } + var lastUpdates = _updateTimestamps.GetMany(spaces.ToArray()); + return lastUpdates.All(lastUpdate => !IsOutdated(lastUpdate as long?, timestamp)); } + [MethodImpl(MethodImplOptions.Synchronized)] public virtual bool[] AreUpToDate(ISet[] spaces, long[] timestamps) { - if (spaces.Length == 0) - return Array.Empty(); - + var results = new bool[spaces.Length]; var allSpaces = new HashSet(); foreach (var sp in spaces) { @@ -146,38 +119,34 @@ public virtual bool[] AreUpToDate(ISet[] spaces, long[] timestamps) if (allSpaces.Count == 0) { - return ArrayHelper.Fill(true, spaces.Length); - } - - Lock.EnterReadLock(); - try - { - var keys = new object[allSpaces.Count]; - var index = 0; - foreach (var space in allSpaces) - { - keys[index++] = space; - } - - index = 0; - var lastUpdatesBySpace = - _updateTimestamps - .GetMany(keys) - .ToDictionary(u => keys[index++], u => u as long?); - - var results = new bool[spaces.Length]; for (var i = 0; i < spaces.Length; i++) { - var timestamp = timestamps[i]; - results[i] = spaces[i].All(space => !IsOutdated(lastUpdatesBySpace[space], timestamp)); + results[i] = true; } return results; } - finally + + var keys = new object[allSpaces.Count]; + var index = 0; + foreach (var space in allSpaces) { - Lock.ExitReadLock(); + keys[index++] = space; } + + index = 0; + var lastUpdatesBySpace = + _updateTimestamps + .GetMany(keys) + .ToDictionary(u => keys[index++], u => u as long?); + + for (var i = 0; i < spaces.Length; i++) + { + var timestamp = timestamps[i]; + results[i] = spaces[i].All(space => !IsOutdated(lastUpdatesBySpace[space], timestamp)); + } + + return results; } // Since v5.3 @@ -188,7 +157,7 @@ public virtual void Destroy() // not the responsibility of this class. } - private static bool IsOutdated(long? lastUpdate, long timestamp) + private bool IsOutdated(long? lastUpdate, long timestamp) { if (!lastUpdate.HasValue) { From d9acfec78f27a6cda6b99b827ad028f22db9468c Mon Sep 17 00:00:00 2001 From: Alexander Zaytsev Date: Mon, 6 May 2019 21:39:59 +1200 Subject: [PATCH 5/5] Code cleanup --- .../Async/Cache/StandardQueryCache.cs | 12 +++++-- .../Async/Cache/UpdateTimestampsCache.cs | 33 ++++++----------- src/NHibernate/Cache/StandardQueryCache.cs | 5 ++- src/NHibernate/Cache/UpdateTimestampsCache.cs | 35 ++++++------------- 4 files changed, 34 insertions(+), 51 deletions(-) diff --git a/src/NHibernate/Async/Cache/StandardQueryCache.cs b/src/NHibernate/Async/Cache/StandardQueryCache.cs index 1063dafb67d..fe7a28afac1 100644 --- a/src/NHibernate/Async/Cache/StandardQueryCache.cs +++ b/src/NHibernate/Async/Cache/StandardQueryCache.cs @@ -464,10 +464,16 @@ private async Task GetResultFromCacheableAsync( } } - protected virtual async Task IsUpToDateAsync(ISet spaces, long timestamp, CancellationToken cancellationToken) + protected virtual Task IsUpToDateAsync(ISet spaces, long timestamp, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); - return spaces.Count == 0 || await (_updateTimestampsCache.IsUpToDateAsync(spaces, timestamp, cancellationToken)).ConfigureAwait(false); + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); + } + if (spaces.Count == 0) + return Task.FromResult(true); + + return _updateTimestampsCache.IsUpToDateAsync(spaces, timestamp, cancellationToken); } } } diff --git a/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs b/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs index 98d6d275326..f97f25401be 100644 --- a/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs +++ b/src/NHibernate/Async/Cache/UpdateTimestampsCache.cs @@ -116,13 +116,9 @@ private Task SetSpacesTimestampAsync(IReadOnlyCollection spaces, long ts if (spaces.Count == 0) return Task.CompletedTask; - var timestamps = new object[spaces.Count]; - for (var i = 0; i < timestamps.Length; i++) - { - timestamps[i] = ts; - } - - return _updateTimestamps.PutManyAsync(spaces.ToArray(), timestamps, cancellationToken); + return _updateTimestamps.PutManyAsync( + spaces.ToArray(), + ArrayHelper.Fill(ts, spaces.Count), cancellationToken); } catch (Exception ex) { @@ -139,7 +135,7 @@ public virtual async Task IsUpToDateAsync(ISet spaces, long timest if (spaces.Count == 0) return true; - var lastUpdates = await (_updateTimestamps.GetManyAsync(spaces.ToArray(), cancellationToken)).ConfigureAwait(false); + var lastUpdates = await (_updateTimestamps.GetManyAsync(spaces.ToArray(), cancellationToken)).ConfigureAwait(false); return lastUpdates.All(lastUpdate => !IsOutdated(lastUpdate as long?, timestamp)); } } @@ -150,7 +146,9 @@ public virtual async Task AreUpToDateAsync(ISet[] spaces, long[] cancellationToken.ThrowIfCancellationRequested(); using (await _areUpToDate.LockAsync()) { - var results = new bool[spaces.Length]; + if (spaces.Length == 0) + return Array.Empty(); + var allSpaces = new HashSet(); foreach (var sp in spaces) { @@ -158,28 +156,17 @@ public virtual async Task AreUpToDateAsync(ISet[] spaces, long[] } if (allSpaces.Count == 0) - { - for (var i = 0; i < spaces.Length; i++) - { - results[i] = true; - } + return ArrayHelper.Fill(true, spaces.Length); - return results; - } + var keys = allSpaces.ToArray(); - var keys = new object[allSpaces.Count]; var index = 0; - foreach (var space in allSpaces) - { - keys[index++] = space; - } - - index = 0; var lastUpdatesBySpace = (await (_updateTimestamps .GetManyAsync(keys, cancellationToken)).ConfigureAwait(false)) .ToDictionary(u => keys[index++], u => u as long?); + var results = new bool[spaces.Length]; for (var i = 0; i < spaces.Length; i++) { var timestamp = timestamps[i]; diff --git a/src/NHibernate/Cache/StandardQueryCache.cs b/src/NHibernate/Cache/StandardQueryCache.cs index 28236beb549..e1867ab5aea 100644 --- a/src/NHibernate/Cache/StandardQueryCache.cs +++ b/src/NHibernate/Cache/StandardQueryCache.cs @@ -551,7 +551,10 @@ private static ICacheAssembler[] GuessTypes(IList cacheable) protected virtual bool IsUpToDate(ISet spaces, long timestamp) { - return spaces.Count == 0 || _updateTimestampsCache.IsUpToDate(spaces, timestamp); + if (spaces.Count == 0) + return true; + + return _updateTimestampsCache.IsUpToDate(spaces, timestamp); } } } diff --git a/src/NHibernate/Cache/UpdateTimestampsCache.cs b/src/NHibernate/Cache/UpdateTimestampsCache.cs index c7270daf3a5..40369e4ac97 100644 --- a/src/NHibernate/Cache/UpdateTimestampsCache.cs +++ b/src/NHibernate/Cache/UpdateTimestampsCache.cs @@ -88,13 +88,9 @@ private void SetSpacesTimestamp(IReadOnlyCollection spaces, long ts) if (spaces.Count == 0) return; - var timestamps = new object[spaces.Count]; - for (var i = 0; i < timestamps.Length; i++) - { - timestamps[i] = ts; - } - - _updateTimestamps.PutMany(spaces.ToArray(), timestamps); + _updateTimestamps.PutMany( + spaces.ToArray(), + ArrayHelper.Fill(ts, spaces.Count)); } [MethodImpl(MethodImplOptions.Synchronized)] @@ -103,14 +99,16 @@ public virtual bool IsUpToDate(ISet spaces, long timestamp /* H2.1 has L if (spaces.Count == 0) return true; - var lastUpdates = _updateTimestamps.GetMany(spaces.ToArray()); + var lastUpdates = _updateTimestamps.GetMany(spaces.ToArray()); return lastUpdates.All(lastUpdate => !IsOutdated(lastUpdate as long?, timestamp)); } [MethodImpl(MethodImplOptions.Synchronized)] public virtual bool[] AreUpToDate(ISet[] spaces, long[] timestamps) { - var results = new bool[spaces.Length]; + if (spaces.Length == 0) + return Array.Empty(); + var allSpaces = new HashSet(); foreach (var sp in spaces) { @@ -118,28 +116,17 @@ public virtual bool[] AreUpToDate(ISet[] spaces, long[] timestamps) } if (allSpaces.Count == 0) - { - for (var i = 0; i < spaces.Length; i++) - { - results[i] = true; - } + return ArrayHelper.Fill(true, spaces.Length); - return results; - } + var keys = allSpaces.ToArray(); - var keys = new object[allSpaces.Count]; var index = 0; - foreach (var space in allSpaces) - { - keys[index++] = space; - } - - index = 0; var lastUpdatesBySpace = _updateTimestamps .GetMany(keys) .ToDictionary(u => keys[index++], u => u as long?); + var results = new bool[spaces.Length]; for (var i = 0; i < spaces.Length; i++) { var timestamp = timestamps[i]; @@ -157,7 +144,7 @@ public virtual void Destroy() // not the responsibility of this class. } - private bool IsOutdated(long? lastUpdate, long timestamp) + private static bool IsOutdated(long? lastUpdate, long timestamp) { if (!lastUpdate.HasValue) {