Skip to content

Commit a086ade

Browse files
fredericDelaportefredericDelaporte
committed
Fix cache build for honoring mapped concurrency and avoid duplicating caches
1 parent 9119548 commit a086ade

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 extension 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
@@ -230,7 +230,6 @@ public SessionFactoryImpl(Configuration cfg, IMapping mapping, Settings settings
230230

231231
#region Persisters
232232

233-
Dictionary<string, ICacheConcurrencyStrategy> caches = new Dictionary<string, ICacheConcurrencyStrategy>();
234233
entityPersisters = new Dictionary<string, IEntityPersister>();
235234
implementorToEntityName = new Dictionary<System.Type, string>();
236235

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

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

378370
if (settings.IsQueryCacheEnabled)
379371
{
380-
updateTimestampsCache = new UpdateTimestampsCache(settings, properties);
381-
queryCache = settings.QueryCacheFactory.GetQueryCache(null, updateTimestampsCache, settings, properties);
372+
var updateTimestampsCacheName = typeof(StandardQueryCache).Name;
373+
updateTimestampsCache = new UpdateTimestampsCache(GetOrBuild(updateTimestampsCacheName));
374+
var queryCacheName = typeof(StandardQueryCache).FullName;
375+
queryCache = settings.QueryCacheFactory.GetQueryCache(
376+
queryCacheName,
377+
updateTimestampsCache,
378+
settings,
379+
properties,
380+
GetOrBuild(queryCacheName));
382381
queryCaches = new ConcurrentDictionary<string, Lazy<IQueryCache>>();
383382
}
384383
else
@@ -1005,6 +1004,25 @@ public ICache GetSecondLevelCacheRegion(string regionName)
10051004
return result;
10061005
}
10071006

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

10441062
public void EvictQueries()

0 commit comments

Comments
 (0)