From 69033cef2d9f82a46a4bcbaf06e28c3f24b45bf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericDelaporte@users.noreply.github.com> Date: Sat, 14 Jul 2018 17:00:52 +0200 Subject: [PATCH 1/2] Reduce cache put batches fragmentation --- src/NHibernate/Async/Cache/CacheBatcher.cs | 64 ++++-------------- src/NHibernate/Cache/CacheBatcher.cs | 79 ++++++++++------------ 2 files changed, 48 insertions(+), 95 deletions(-) diff --git a/src/NHibernate/Async/Cache/CacheBatcher.cs b/src/NHibernate/Async/Cache/CacheBatcher.cs index de7fa2228b9..4a1d7475c96 100644 --- a/src/NHibernate/Async/Cache/CacheBatcher.cs +++ b/src/NHibernate/Async/Cache/CacheBatcher.cs @@ -8,7 +8,9 @@ //------------------------------------------------------------------------------ +using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using NHibernate.Engine; using NHibernate.Persister.Collection; using NHibernate.Persister.Entity; @@ -21,59 +23,13 @@ public sealed partial class CacheBatcher { /// - /// Adds a put operation to the batch. If the batch size reached the persister batch - /// size, the batch will be executed. - /// - /// The entity persister. - /// The data to put in the cache. - /// A cancellation token that can be used to cancel the work - internal async Task AddToBatchAsync(IEntityPersister persister, CachePutData data, CancellationToken cancellationToken) - { - cancellationToken.ThrowIfCancellationRequested(); - if (ShouldExecuteBatch(persister, _putBatch)) - { - await (ExecuteBatchAsync(cancellationToken)).ConfigureAwait(false); - _currentPersister = persister; - _currentBatch = _putBatch = new CachePutBatch(_session, persister.Cache); - } - if (Log.IsDebugEnabled()) - { - Log.Debug("Adding a put operation to batch for entity {0} and key {1}", persister.EntityName, data.Key); - } - _putBatch.Add(data); - } - - /// - /// Adds a put operation to the batch. If the batch size reached the persister batch - /// size, the batch will be executed. - /// - /// The collection persister. - /// The data to put in the cache. - /// A cancellation token that can be used to cancel the work - internal async Task AddToBatchAsync(ICollectionPersister persister, CachePutData data, CancellationToken cancellationToken) - { - cancellationToken.ThrowIfCancellationRequested(); - if (ShouldExecuteBatch(persister, _putBatch)) - { - await (ExecuteBatchAsync(cancellationToken)).ConfigureAwait(false); - _currentPersister = persister; - _currentBatch = _putBatch = new CachePutBatch(_session, persister.Cache); - } - if (Log.IsDebugEnabled()) - { - Log.Debug("Adding a put operation to batch for collection role {0} and key {1}", persister.Role, data.Key); - } - _putBatch.Add(data); - } - - /// - /// Executes the current batch. + /// Executes the pending batches. /// /// A cancellation token that can be used to cancel the work internal async Task ExecuteBatchAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - if (_currentBatch == null || _currentBatch.BatchSize == 0) + if (_putBatches.Count == 0) { return; } @@ -85,10 +41,18 @@ internal async Task ExecuteBatchAsync(CancellationToken cancellationToken) { duration = Stopwatch.StartNew(); } - await (_currentBatch.ExecuteAsync(cancellationToken)).ConfigureAwait(false); + + foreach (var batch in _putBatches.Values) + { + await (batch.ExecuteAsync(cancellationToken)).ConfigureAwait(false); + } + if (Log.IsDebugEnabled() && duration != null) { - Log.Debug("ExecuteBatch for {0} keys took {1} ms", _currentBatch.BatchSize, duration.ElapsedMilliseconds); + Log.Debug( + "ExecuteBatch for {0} batches totaling {1} keys took {2} ms", + _putBatches.Count, _putBatches.Values.Sum(b => b.BatchSize), + duration.ElapsedMilliseconds); } } finally diff --git a/src/NHibernate/Cache/CacheBatcher.cs b/src/NHibernate/Cache/CacheBatcher.cs index acf96431018..f94a2377e5b 100644 --- a/src/NHibernate/Cache/CacheBatcher.cs +++ b/src/NHibernate/Cache/CacheBatcher.cs @@ -1,4 +1,6 @@ -using System.Diagnostics; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; using NHibernate.Engine; using NHibernate.Persister.Collection; using NHibernate.Persister.Entity; @@ -6,16 +8,13 @@ namespace NHibernate.Cache { /// - /// A batcher for batching operations of , where the batch size is retrieved - /// from an or . - /// When a different persister or a different operation is added to the batch, the current batch will be executed. + /// A batcher for batching operations of . /// public sealed partial class CacheBatcher { - private CachePutBatch _putBatch; + private readonly Dictionary _putBatches = + new Dictionary(); private readonly ISessionImplementor _session; - private AbstractCacheBatch _currentBatch; - private object _currentPersister; private static readonly INHibernateLogger Log = NHibernateLogger.For(typeof(CacheBatcher)); @@ -25,53 +24,50 @@ internal CacheBatcher(ISessionImplementor session) } /// - /// Adds a put operation to the batch. If the batch size reached the persister batch - /// size, the batch will be executed. + /// Adds a put operation to the batch. /// /// The entity persister. /// The data to put in the cache. internal void AddToBatch(IEntityPersister persister, CachePutData data) { - if (ShouldExecuteBatch(persister, _putBatch)) - { - ExecuteBatch(); - _currentPersister = persister; - _currentBatch = _putBatch = new CachePutBatch(_session, persister.Cache); - } if (Log.IsDebugEnabled()) { Log.Debug("Adding a put operation to batch for entity {0} and key {1}", persister.EntityName, data.Key); } - _putBatch.Add(data); + AddToBatch(persister.Cache, data); } /// - /// Adds a put operation to the batch. If the batch size reached the persister batch - /// size, the batch will be executed. + /// Adds a put operation to the batch. /// /// The collection persister. /// The data to put in the cache. internal void AddToBatch(ICollectionPersister persister, CachePutData data) { - if (ShouldExecuteBatch(persister, _putBatch)) - { - ExecuteBatch(); - _currentPersister = persister; - _currentBatch = _putBatch = new CachePutBatch(_session, persister.Cache); - } if (Log.IsDebugEnabled()) { Log.Debug("Adding a put operation to batch for collection role {0} and key {1}", persister.Role, data.Key); } - _putBatch.Add(data); + AddToBatch(persister.Cache, data); + } + + private void AddToBatch(ICacheConcurrencyStrategy cache, CachePutData data) + { + if (!_putBatches.TryGetValue(cache, out var cachePutBatch)) + { + cachePutBatch = new CachePutBatch(_session, cache); + _putBatches.Add(cache, cachePutBatch); + } + + cachePutBatch.Add(data); } /// - /// Executes the current batch. + /// Executes the pending batches. /// internal void ExecuteBatch() { - if (_currentBatch == null || _currentBatch.BatchSize == 0) + if (_putBatches.Count == 0) { return; } @@ -83,10 +79,18 @@ internal void ExecuteBatch() { duration = Stopwatch.StartNew(); } - _currentBatch.Execute(); + + foreach (var batch in _putBatches.Values) + { + batch.Execute(); + } + if (Log.IsDebugEnabled() && duration != null) { - Log.Debug("ExecuteBatch for {0} keys took {1} ms", _currentBatch.BatchSize, duration.ElapsedMilliseconds); + Log.Debug( + "ExecuteBatch for {0} batches totaling {1} keys took {2} ms", + _putBatches.Count, _putBatches.Values.Sum(b => b.BatchSize), + duration.ElapsedMilliseconds); } } finally @@ -100,22 +104,7 @@ internal void ExecuteBatch() /// internal void Cleanup() { - _putBatch = null; - - _currentBatch = null; - _currentPersister = null; - } - - private bool ShouldExecuteBatch(IEntityPersister persister, AbstractCacheBatch batch) - { - return batch != _currentBatch || _currentPersister != persister || - _currentBatch.BatchSize >= persister.GetBatchSize(); - } - - private bool ShouldExecuteBatch(ICollectionPersister persister, AbstractCacheBatch batch) - { - return batch != _currentBatch || _currentPersister != persister || - _currentBatch.BatchSize >= persister.GetBatchSize(); + _putBatches.Clear(); } } } From 947629941d6917f5f721694de26dbbab574c44bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericDelaporte@users.noreply.github.com> Date: Sun, 15 Jul 2018 12:19:13 +0200 Subject: [PATCH 2/2] fixup! Reduce cache put batches fragmentation Account the change in tests --- .../Async/CacheTest/BatchableCacheFixture.cs | 15 +++++++-------- .../CacheTest/BatchableCacheFixture.cs | 15 +++++++-------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs b/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs index c2baef09b8d..e2d4c3db782 100644 --- a/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs +++ b/src/NHibernate.Test/Async/CacheTest/BatchableCacheFixture.cs @@ -450,15 +450,16 @@ public async Task MultiplePutReadWriteTestAsync() ids.AddRange(items.OrderBy(o => o.Id).Select(o => o.Id)); await (tx.CommitAsync()); } - Assert.That(cache.PutCalls, Has.Count.EqualTo(0)); - Assert.That(cache.GetMultipleCalls, Has.Count.EqualTo(2)); + Assert.That(cache.PutCalls, Has.Count.EqualTo(0), "Cache put"); + Assert.That(cache.PutMultipleCalls, Has.Count.EqualTo(1), "Cache put many"); + // Lock get + Assert.That(cache.GetMultipleCalls, Has.Count.EqualTo(1), "Cache get many"); AssertEquivalent( ids, new[] { - new[] {0, 1, 2}, - new[] {3, 4, 5} + new[] {0, 1, 2, 3, 4, 5} }, cache.PutMultipleCalls ); @@ -466,8 +467,7 @@ public async Task MultiplePutReadWriteTestAsync() ids, new[] { - new[] {0, 1, 2}, - new[] {3, 4, 5} + new[] {0, 1, 2, 3, 4, 5} }, cache.LockMultipleCalls ); @@ -475,8 +475,7 @@ public async Task MultiplePutReadWriteTestAsync() ids, new[] { - new[] {0, 1, 2}, - new[] {3, 4, 5} + new[] {0, 1, 2, 3, 4, 5} }, cache.UnlockMultipleCalls ); diff --git a/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs b/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs index 9e166ef6509..82b90d5f773 100644 --- a/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs +++ b/src/NHibernate.Test/CacheTest/BatchableCacheFixture.cs @@ -438,15 +438,16 @@ public void MultiplePutReadWriteTest() ids.AddRange(items.OrderBy(o => o.Id).Select(o => o.Id)); tx.Commit(); } - Assert.That(cache.PutCalls, Has.Count.EqualTo(0)); - Assert.That(cache.GetMultipleCalls, Has.Count.EqualTo(2)); + Assert.That(cache.PutCalls, Has.Count.EqualTo(0), "Cache put"); + Assert.That(cache.PutMultipleCalls, Has.Count.EqualTo(1), "Cache put many"); + // Lock get + Assert.That(cache.GetMultipleCalls, Has.Count.EqualTo(1), "Cache get many"); AssertEquivalent( ids, new[] { - new[] {0, 1, 2}, - new[] {3, 4, 5} + new[] {0, 1, 2, 3, 4, 5} }, cache.PutMultipleCalls ); @@ -454,8 +455,7 @@ public void MultiplePutReadWriteTest() ids, new[] { - new[] {0, 1, 2}, - new[] {3, 4, 5} + new[] {0, 1, 2, 3, 4, 5} }, cache.LockMultipleCalls ); @@ -463,8 +463,7 @@ public void MultiplePutReadWriteTest() ids, new[] { - new[] {0, 1, 2}, - new[] {3, 4, 5} + new[] {0, 1, 2, 3, 4, 5} }, cache.UnlockMultipleCalls );