Skip to content

Commit 14991c7

Browse files
fredericDelaportefredericDelaporte
committed
Fix cache build for honoring mapped concurrency and avoid duplicating caches
1 parent 92cf18d commit 14991c7

File tree

6 files changed

+191
-61
lines changed

6 files changed

+191
-61
lines changed

‎src/NHibernate/Cache/CacheFactory.cs‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
1+
using System;
22
using NHibernate.Cfg;
33
using System.Collections.Generic;
44

@@ -9,7 +9,7 @@ namespace NHibernate.Cache
99
/// </summary>
1010
public static class CacheFactory
1111
{
12-
private static readonly INHibernateLogger log = NHibernateLogger.For(typeof(CacheFactory));
12+
private static readonly INHibernateLogger Log = NHibernateLogger.For(typeof(CacheFactory));
1313

1414
public const string ReadOnly = "read-only";
1515
public const string ReadWrite = "read-write";
@@ -30,17 +30,41 @@ public static class CacheFactory
3030
/// <param name="settings">Used to retrieve the global cache region prefix.</param>
3131
/// <param name="properties">Properties the cache provider can use to configure the cache.</param>
3232
/// <returns>An <see cref="ICacheConcurrencyStrategy"/> to use for this object in the <see cref="ICache"/>.</returns>
33-
public static ICacheConcurrencyStrategy CreateCache(string usage, string name, bool mutable, Settings settings,
34-
IDictionary<string,string> properties)
33+
// Since v5.1
34+
[Obsolete("Please use overload with an ICache parameter.")]
35+
public static ICacheConcurrencyStrategy CreateCache(
36+
string usage,
37+
string name,
38+
bool mutable,
39+
Settings settings,
40+
IDictionary<string, string> properties)
3541
{
36-
if (usage == null || !settings.IsSecondLevelCacheEnabled) return null; //no cache
42+
return CreateCache(
43+
usage, name, mutable, settings,
44+
r => settings.CacheProvider.BuildCache(r, properties));
45+
}
3746

38-
string prefix = settings.CacheRegionPrefix;
39-
if (prefix != null) name = prefix + '.' + name;
47+
/// <summary>
48+
/// Creates an <see cref="ICacheConcurrencyStrategy"/> from the parameters.
49+
/// </summary>
50+
/// <param name="usage">The name of the strategy that <see cref="ICacheProvider"/> should use for the class.</param>
51+
/// <param name="name">The name of the class the strategy is being created for.</param>
52+
/// <param name="mutable"><see langword="true" /> if the object being stored in the cache is mutable.</param>
53+
/// <param name="settings">Used to retrieve the global cache region prefix.</param>
54+
/// <param name="regionCacheGetter">The delegate for obtaining the <see cref="ICache" /> to use for the region.</param>
55+
/// <returns>An <see cref="ICacheConcurrencyStrategy"/> to use for this object in the <see cref="ICache"/>.</returns>
56+
public static ICacheConcurrencyStrategy CreateCache(
57+
string usage,
58+
string name,
59+
bool mutable,
60+
Settings settings,
61+
Func<string, ICache> regionCacheGetter)
62+
{
63+
if (usage == null || !settings.IsSecondLevelCacheEnabled) return null; //no cache
4064

41-
if (log.IsDebugEnabled())
65+
if (Log.IsDebugEnabled())
4266
{
43-
log.Debug("cache for: {0} usage strategy: {1}", name, usage);
67+
Log.Debug("cache for: {0} usage strategy: {1}", name, usage);
4468
}
4569

4670
ICacheConcurrencyStrategy ccs;
@@ -49,7 +73,7 @@ public static ICacheConcurrencyStrategy CreateCache(string usage, string name, b
4973
case ReadOnly:
5074
if (mutable)
5175
{
52-
log.Warn("read-only cache configured for mutable: {0}", name);
76+
Log.Warn("read-only cache configured for mutable: {0}", name);
5377
}
5478
ccs = new ReadOnlyCache();
5579
break;
@@ -59,18 +83,18 @@ public static ICacheConcurrencyStrategy CreateCache(string usage, string name, b
5983
case NonstrictReadWrite:
6084
ccs = new NonstrictReadWriteCache();
6185
break;
62-
//case CacheFactory.Transactional:
63-
// ccs = new TransactionalCache();
64-
// break;
86+
//case CacheFactory.Transactional:
87+
// ccs = new TransactionalCache();
88+
// break;
6589
default:
6690
throw new MappingException(
67-
"cache usage attribute should be read-write, read-only, nonstrict-read-write, or transactional");
91+
"cache usage attribute should be read-write, read-only or nonstrict-read-write");
6892
}
6993

7094
ICache impl;
7195
try
7296
{
73-
impl = settings.CacheProvider.BuildCache(name, properties);
97+
impl = regionCacheGetter(name);
7498
}
7599
catch (CacheException e)
76100
{

‎src/NHibernate/Cache/IQueryCacheFactory.cs‎

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections.Generic;
23
using NHibernate.Cfg;
34

@@ -9,7 +10,45 @@ namespace NHibernate.Cache
910
/// </summary>
1011
public interface IQueryCacheFactory
1112
{
12-
IQueryCache GetQueryCache(string regionName, UpdateTimestampsCache updateTimestampsCache, Settings settings,
13-
IDictionary<string, string> props);
13+
// Since v5.1
14+
[Obsolete("Please use extension overload with an ICache parameter.")]
15+
IQueryCache GetQueryCache(
16+
string regionName,
17+
UpdateTimestampsCache updateTimestampsCache,
18+
Settings settings,
19+
IDictionary<string, string> props);
1420
}
15-
}
21+
22+
// 6.0 TODO: move to interface and purge parameters usefull only for building the regionCache.
23+
// (Only updateTimestampsCache and regionCache must then remain. Leaving props too for allowing custom factories
24+
// to have configuration parameters.)
25+
public static class QueryCacheFactoryExtension
26+
{
27+
/// <summary>
28+
/// Build a query cache.
29+
/// </summary>
30+
/// <param name="factory">The query cache factory.</param>
31+
/// <param name="regionName">The cache region.</param>
32+
/// <param name="updateTimestampsCache">The cache of updates timestamps.</param>
33+
/// <param name="settings">The NHibernate settings.</param>
34+
/// <param name="props">The NHibernate settings properties.</param>
35+
/// <param name="regionCache">The <see cref="ICache" /> to use for the region.</param>
36+
/// <returns>A query cache.</returns>
37+
public static IQueryCache GetQueryCache(
38+
this IQueryCacheFactory factory,
39+
string regionName,
40+
UpdateTimestampsCache updateTimestampsCache,
41+
Settings settings,
42+
IDictionary<string, string> props,
43+
ICache regionCache)
44+
{
45+
if (factory is StandardQueryCacheFactory standardFactory)
46+
{
47+
return standardFactory.GetQueryCache(updateTimestampsCache, props, regionCache);
48+
}
49+
#pragma warning disable 618
50+
return factory.GetQueryCache(regionName, updateTimestampsCache, settings, props);
51+
#pragma warning restore 618
52+
}
53+
}
54+
}

‎src/NHibernate/Cache/StandardQueryCache.cs‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,39 @@ public partial class StandardQueryCache : IQueryCache
2222
private readonly string _regionName;
2323
private readonly UpdateTimestampsCache _updateTimestampsCache;
2424

25-
public StandardQueryCache(Settings settings, IDictionary<string, string> props, UpdateTimestampsCache updateTimestampsCache, string regionName)
25+
// Since v5.1
26+
[Obsolete("Please use overload with an ICache parameter.")]
27+
public StandardQueryCache(
28+
Settings settings,
29+
IDictionary<string, string> props,
30+
UpdateTimestampsCache updateTimestampsCache,
31+
string regionName)
32+
: this(
33+
updateTimestampsCache,
34+
settings.CacheProvider.BuildCache(
35+
(!string.IsNullOrEmpty(settings.CacheRegionPrefix) ? settings.CacheRegionPrefix + '.' : "") +
36+
(regionName ?? typeof (StandardQueryCache).FullName),
37+
props))
2638
{
27-
if (regionName == null)
28-
regionName = typeof (StandardQueryCache).FullName;
39+
}
2940

30-
String prefix = settings.CacheRegionPrefix;
31-
if (!string.IsNullOrEmpty(prefix))
32-
regionName = prefix + '.' + regionName;
41+
/// <summary>
42+
/// Build a query cache.
43+
/// </summary>
44+
/// <param name="updateTimestampsCache">The cache of updates timestamps.</param>
45+
/// <param name="regionCache">The <see cref="ICache" /> to use for the region.</param>
46+
public StandardQueryCache(
47+
UpdateTimestampsCache updateTimestampsCache,
48+
ICache regionCache)
49+
{
50+
if (regionCache == null)
51+
throw new ArgumentNullException(nameof(regionCache));
3352

34-
Log.Info("starting query cache at region: {0}", regionName);
53+
_regionName = regionCache.RegionName;
54+
Log.Info("starting query cache at region: {0}", _regionName);
3555

36-
_queryCache = settings.CacheProvider.BuildCache(regionName, props);
56+
_queryCache = regionCache;
3757
_updateTimestampsCache = updateTimestampsCache;
38-
_regionName = regionName;
3958
}
4059

4160
#region IQueryCache Members

‎src/NHibernate/Cache/StandardQueryCacheFactory.cs‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections.Generic;
23
using NHibernate.Cfg;
34

@@ -9,12 +10,29 @@ namespace NHibernate.Cache
910
/// </summary>
1011
public class StandardQueryCacheFactory : IQueryCacheFactory
1112
{
13+
// Since v5.1
14+
[Obsolete("Please use overload with an ICache parameter.")]
1215
public IQueryCache GetQueryCache(string regionName,
1316
UpdateTimestampsCache updateTimestampsCache,
1417
Settings settings,
1518
IDictionary<string, string> props)
1619
{
1720
return new StandardQueryCache(settings, props, updateTimestampsCache, regionName);
1821
}
22+
23+
/// <summary>
24+
/// Build a query cache.
25+
/// </summary>
26+
/// <param name="updateTimestampsCache">The cache of updates timestamps.</param>
27+
/// <param name="props">The NHibernate settings properties.</param>
28+
/// <param name="regionCache">The <see cref="ICache" /> to use for the region.</param>
29+
/// <returns>A query cache.</returns>
30+
public virtual IQueryCache GetQueryCache(
31+
UpdateTimestampsCache updateTimestampsCache,
32+
IDictionary<string, string> props,
33+
ICache regionCache)
34+
{
35+
return new StandardQueryCache(updateTimestampsCache, regionCache);
36+
}
1937
}
20-
}
38+
}

‎src/NHibernate/Cache/UpdateTimestampsCache.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ public virtual void Clear()
2626
updateTimestamps.Clear();
2727
}
2828

29+
// Since v5.1
30+
[Obsolete("Please use overload with an ICache parameter.")]
2931
public UpdateTimestampsCache(Settings settings, IDictionary<string, string> props)
3032
{
3133
string prefix = settings.CacheRegionPrefix;
@@ -34,6 +36,16 @@ public UpdateTimestampsCache(Settings settings, IDictionary<string, string> prop
3436
updateTimestamps = settings.CacheProvider.BuildCache(regionName, props);
3537
}
3638

39+
/// <summary>
40+
/// Build the update timestamps cache.
41+
/// </summary>
42+
/// <param name="cache">The <see cref="ICache" /> to use.</param>
43+
public UpdateTimestampsCache(ICache cache)
44+
{
45+
log.Info("starting update timestamps cache at region: {0}", cache.RegionName);
46+
updateTimestamps = cache;
47+
}
48+
3749
//Since v5.1
3850
[Obsolete("Please use PreInvalidate(IReadOnlyCollection<string>) instead.")]
3951
public void PreInvalidate(object[] spaces)

‎src/NHibernate/Impl/SessionFactoryImpl.cs‎

Lines changed: 51 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,6 @@ public SessionFactoryImpl(Configuration cfg, IMapping mapping, Settings settings
232232

233233
#region Persisters
234234

235-
Dictionary<string, ICacheConcurrencyStrategy> caches = new Dictionary<string, ICacheConcurrencyStrategy>();
236235
entityPersisters = new Dictionary<string, IEntityPersister>();
237236
implementorToEntityName = new Dictionary<System.Type, string>();
238237

@@ -241,22 +240,15 @@ public SessionFactoryImpl(Configuration cfg, IMapping mapping, Settings settings
241240
foreach (PersistentClass model in cfg.ClassMappings)
242241
{
243242
model.PrepareTemporaryTables(mapping, settings.Dialect);
244-
string cacheRegion = model.RootClazz.CacheRegionName;
245-
ICacheConcurrencyStrategy cache;
246-
if (!caches.TryGetValue(cacheRegion, out cache))
247-
{
248-
cache =
249-
CacheFactory.CreateCache(model.CacheConcurrencyStrategy, cacheRegion, model.IsMutable, settings, properties);
250-
if (cache != null)
251-
{
252-
caches.Add(cacheRegion, cache);
253-
if (!allCacheRegions.TryAdd(cache.RegionName, cache.Cache))
254-
{
255-
throw new HibernateException("An item with the same key has already been added to allCacheRegions.");
256-
}
257-
}
258-
}
259-
IEntityPersister cp = PersisterFactory.CreateClassPersister(model, cache, this, mapping);
243+
var cacheRegion = model.RootClazz.CacheRegionName;
244+
var strategy = model.CacheConcurrencyStrategy;
245+
var cache = CacheFactory.CreateCache(
246+
strategy,
247+
cacheRegion,
248+
model.IsMutable,
249+
settings,
250+
GetOrBuild);
251+
var cp = PersisterFactory.CreateClassPersister(model, cache, this, mapping);
260252
entityPersisters[model.EntityName] = cp;
261253
classMeta[model.EntityName] = cp.ClassMetadata;
262254

@@ -271,14 +263,14 @@ public SessionFactoryImpl(Configuration cfg, IMapping mapping, Settings settings
271263
collectionPersisters = new Dictionary<string, ICollectionPersister>();
272264
foreach (Mapping.Collection model in cfg.CollectionMappings)
273265
{
274-
ICacheConcurrencyStrategy cache =
275-
CacheFactory.CreateCache(model.CacheConcurrencyStrategy, model.CacheRegionName, model.Owner.IsMutable, settings,
276-
properties);
277-
if (cache != null)
278-
{
279-
allCacheRegions[cache.RegionName] = cache.Cache;
280-
}
281-
ICollectionPersister persister = PersisterFactory.CreateCollectionPersister(model, cache, this);
266+
var cacheRegion = model.CacheRegionName;
267+
var cache = CacheFactory.CreateCache(
268+
model.CacheConcurrencyStrategy,
269+
cacheRegion,
270+
model.Owner.IsMutable,
271+
settings,
272+
GetOrBuild);
273+
var persister = PersisterFactory.CreateCollectionPersister(model, cache, this);
282274
collectionPersisters[model.Role] = persister;
283275
IType indexType = persister.IndexType;
284276
if (indexType != null && indexType.IsAssociationType && !indexType.IsAnyType)
@@ -379,8 +371,15 @@ public SessionFactoryImpl(Configuration cfg, IMapping mapping, Settings settings
379371

380372
if (settings.IsQueryCacheEnabled)
381373
{
382-
updateTimestampsCache = new UpdateTimestampsCache(settings, properties);
383-
queryCache = settings.QueryCacheFactory.GetQueryCache(null, updateTimestampsCache, settings, properties);
374+
var updateTimestampsCacheName = typeof(StandardQueryCache).Name;
375+
updateTimestampsCache = new UpdateTimestampsCache(GetOrBuild(updateTimestampsCacheName));
376+
var queryCacheName = typeof(StandardQueryCache).FullName;
377+
queryCache = settings.QueryCacheFactory.GetQueryCache(
378+
queryCacheName,
379+
updateTimestampsCache,
380+
settings,
381+
properties,
382+
GetOrBuild(queryCacheName));
384383
queryCaches = new ConcurrentDictionary<string, Lazy<IQueryCache>>();
385384
}
386385
else
@@ -1007,6 +1006,25 @@ public ICache GetSecondLevelCacheRegion(string regionName)
10071006
return result;
10081007
}
10091008

1009+
/// <summary>
1010+
/// Get an existing <see cref="ICache"/> or build a new one if not already existing.
1011+
/// </summary>
1012+
/// <param name="cacheRegion">The (unprefixed) cache region of the cache to get or build.</param>
1013+
/// <returns>A cache.</returns>
1014+
private ICache GetOrBuild(string cacheRegion)
1015+
{
1016+
// If run concurrently for the same region, this may built many caches for the same region.
1017+
// Currently only GetQueryCache may be run concurrently, and its implementation prevents
1018+
// concurrent creation call for the same region, so this will not happen.
1019+
// Otherwise the dictionary will have to be changed for using a lazy, see
1020+
// https://stackoverflow.com/a/31637510/1178314
1021+
var prefix = settings.CacheRegionPrefix;
1022+
if (!string.IsNullOrEmpty(prefix))
1023+
cacheRegion = prefix + '.' + cacheRegion;
1024+
return allCacheRegions.GetOrAdd(cacheRegion,
1025+
cr => settings.CacheProvider.BuildCache(cr, properties));
1026+
}
1027+
10101028
/// <summary> Statistics SPI</summary>
10111029
public IStatisticsImplementor StatisticsImplementor
10121030
{
@@ -1035,12 +1053,12 @@ public IQueryCache GetQueryCache(string cacheRegion)
10351053
return queryCaches.GetOrAdd(
10361054
cacheRegion,
10371055
cr => new Lazy<IQueryCache>(
1038-
() =>
1039-
{
1040-
var currentQueryCache = settings.QueryCacheFactory.GetQueryCache(cr, updateTimestampsCache, settings, properties);
1041-
allCacheRegions[currentQueryCache.RegionName] = currentQueryCache.Cache;
1042-
return currentQueryCache;
1043-
})).Value;
1056+
() => settings.QueryCacheFactory.GetQueryCache(
1057+
cr,
1058+
updateTimestampsCache,
1059+
settings,
1060+
properties,
1061+
GetOrBuild(cr)))).Value;
10441062
}
10451063

10461064
public void EvictQueries()

0 commit comments

Comments
 (0)