yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

U4-4931 - fixing & refactoring

Browse Source
This commit is contained in:
Stephan
2014-06-28 12:09:54 +02:00
parent 75e003185a
commit e6986432df
7 changed files with 205 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
src/Umbraco.Core/Cache/DictionaryCacheProviderBase.cs
View File

@@ -3,7 +3,6 @@ using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading;
namespace Umbraco.Core.Cache
{
@@ -12,7 +11,8 @@ namespace Umbraco.Core.Cache
// prefix cache keys so we know which one are ours
protected const string CacheItemPrefix = "umbrtmche";
protected readonly ReaderWriterLockSlim Locker = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);
// an object that represent a value that has not been created yet
protected readonly object ValueNotCreated = new object();
// manupulate the underlying cache entries
// these *must* be called from within the appropriate locks
@@ -21,15 +21,22 @@ namespace Umbraco.Core.Cache
protected abstract void RemoveEntry(string key);
protected abstract object GetEntry(string key);
// read-write lock the underlying cache
protected abstract IDisposable ReadLock { get; }
protected abstract IDisposable WriteLock { get; }
protected string GetCacheKey(string key)
{
return string.Format("{0}-{1}", CacheItemPrefix, key);
}
protected object GetSafeLazyValue(Lazy<object> lazy)
protected object GetSafeLazyValue(Lazy<object> lazy, bool onlyIfValueIsCreated = false)
{
try
{
// if onlyIfValueIsCreated, do not trigger value creation
// must return something, though, to differenciate from null values
if (onlyIfValueIsCreated && lazy.IsValueCreated == false) return ValueNotCreated;
return lazy.Value;
}
catch
@@ -42,7 +49,7 @@ namespace Umbraco.Core.Cache
public virtual void ClearAllCache()
{
using (new WriteLock(Locker))
using (WriteLock)
{
foreach (var entry in GetDictionaryEntries()
.ToArray())
@@ -52,23 +59,24 @@ namespace Umbraco.Core.Cache
public virtual void ClearCacheItem(string key)
{
using (new WriteLock(Locker))
var cacheKey = GetCacheKey(key);
using (WriteLock)
{
var cacheKey = GetCacheKey(key);
RemoveEntry(cacheKey);
}
}
public virtual void ClearCacheObjectTypes(string typeName)
{
using (new WriteLock(Locker))
using (WriteLock)
{
foreach (var entry in GetDictionaryEntries()
.Where(x =>
{
// entry.Value is Lazy<object> and not null, its value may be null
// remove null values as well, does not hurt
var value = GetSafeLazyValue((Lazy<object>) x.Value); // return exceptions as null
// get non-created as NonCreatedValue & exceptions as null
var value = GetSafeLazyValue((Lazy<object>)x.Value, true);
return value == null || value.GetType().ToString().InvariantEquals(typeName);
})
.ToArray())
@@ -79,7 +87,7 @@ namespace Umbraco.Core.Cache
public virtual void ClearCacheObjectTypes<T>()
{
var typeOfT = typeof(T);
using (new WriteLock(Locker))
using (WriteLock)
{
foreach (var entry in GetDictionaryEntries()
.Where(x =>
@@ -87,7 +95,8 @@ namespace Umbraco.Core.Cache
// entry.Value is Lazy<object> and not null, its value may be null
// remove null values as well, does not hurt
// compare on exact type, don't use "is"
var value = GetSafeLazyValue((Lazy<object>)x.Value); // return exceptions as null
// get non-created as NonCreatedValue & exceptions as null
var value = GetSafeLazyValue((Lazy<object>)x.Value, true);
return value == null || value.GetType() == typeOfT;
})
.ToArray())
@@ -99,7 +108,7 @@ namespace Umbraco.Core.Cache
{
var typeOfT = typeof(T);
var plen = CacheItemPrefix.Length + 1;
using (new WriteLock(Locker))
using (WriteLock)
{
foreach (var entry in GetDictionaryEntries()
.Where(x =>
@@ -107,7 +116,8 @@ namespace Umbraco.Core.Cache
// entry.Value is Lazy<object> and not null, its value may be null
// remove null values as well, does not hurt
// compare on exact type, don't use "is"
var value = GetSafeLazyValue((Lazy<object>)x.Value); // return exceptions as null
// get non-created as NonCreatedValue & exceptions as null
var value = GetSafeLazyValue((Lazy<object>)x.Value, true);
if (value == null) return true;
return value.GetType() == typeOfT
// run predicate on the 'public key' part only, ie without prefix
@@ -120,7 +130,7 @@ namespace Umbraco.Core.Cache
public virtual void ClearCacheByKeySearch(string keyStartsWith)
{
var plen = CacheItemPrefix.Length + 1;
using (new WriteLock(Locker))
using (WriteLock)
{
foreach (var entry in GetDictionaryEntries()
.Where(x => ((string)x.Key).Substring(plen).InvariantStartsWith(keyStartsWith))
@@ -132,7 +142,7 @@ namespace Umbraco.Core.Cache
public virtual void ClearCacheByKeyExpression(string regexString)
{
var plen = CacheItemPrefix.Length + 1;
using (new WriteLock(Locker))
using (WriteLock)
{
foreach (var entry in GetDictionaryEntries()
.Where(x => Regex.IsMatch(((string)x.Key).Substring(plen), regexString))
@@ -148,38 +158,43 @@ namespace Umbraco.Core.Cache
public virtual IEnumerable<object> GetCacheItemsByKeySearch(string keyStartsWith)
{
var plen = CacheItemPrefix.Length + 1;
using (new ReadLock(Locker))
IEnumerable<DictionaryEntry> entries;
using (ReadLock)
{
return GetDictionaryEntries()
entries = GetDictionaryEntries()
.Where(x => ((string)x.Key).Substring(plen).InvariantStartsWith(keyStartsWith))
.Select(x => GetSafeLazyValue((Lazy<object>)x.Value)) // return exceptions as null
.Where(x => x != null) // backward compat, don't store null values in the cache
.ToList();
.ToArray(); // evaluate while locked
}
return entries
.Select(x => GetSafeLazyValue((Lazy<object>)x.Value)) // return exceptions as null
.Where(x => x != null); // backward compat, don't store null values in the cache
}
public virtual IEnumerable<object> GetCacheItemsByKeyExpression(string regexString)
{
const string prefix = CacheItemPrefix + "-";
var plen = prefix.Length;
using (new ReadLock(Locker))
IEnumerable<DictionaryEntry> entries;
using (ReadLock)
{
return GetDictionaryEntries()
entries = GetDictionaryEntries()
.Where(x => Regex.IsMatch(((string)x.Key).Substring(plen), regexString))
.Select(x => GetSafeLazyValue((Lazy<object>)x.Value)) // return exceptions as null
.Where(x => x != null) // backward compat, don't store null values in the cache
.ToList();
.ToArray(); // evaluate while locked
}
return entries
.Select(x => GetSafeLazyValue((Lazy<object>)x.Value)) // return exceptions as null
.Where(x => x != null); // backward compat, don't store null values in the cache
}
public virtual object GetCacheItem(string cacheKey)
{
cacheKey = GetCacheKey(cacheKey);
using (new ReadLock(Locker))
Lazy<object> result;
using (ReadLock)
{
var result = GetEntry(cacheKey) as Lazy<object>; // null if key not found
return result == null ? null : GetSafeLazyValue(result); // return exceptions as null
result = GetEntry(cacheKey) as Lazy<object>; // null if key not found
}
return result == null ? null : GetSafeLazyValue(result); // return exceptions as null
}
public abstract object GetCacheItem(string cacheKey, Func<object> getCacheItem);

87
src/Umbraco.Core/Cache/HttpRequestCacheProvider.cs
View File

@@ -11,37 +11,91 @@ namespace Umbraco.Core.Cache
/// </summary>
internal class HttpRequestCacheProvider : DictionaryCacheProviderBase
{
private readonly Func<HttpContextBase> _context;
// context provider
// the idea is that there is only one, application-wide HttpRequestCacheProvider instance,
// that is initialized with a method that returns the "current" context.
// NOTE
// but then it is initialized with () => new HttpContextWrapper(HttpContent.Current)
// which is higly inefficient because it creates a new wrapper each time we refer to _context()
// so replace it with _context1 and _context2 below + a way to get context.Items.
//private readonly Func<HttpContextBase> _context;
public HttpRequestCacheProvider(HttpContext context)
// NOTE
// and then in almost 100% cases _context2 will be () => HttpContext.Current
// so why not bring that logic in here and fallback on to HttpContext.Current when
// _context1 is null?
//private readonly HttpContextBase _context1;
//private readonly Func<HttpContext> _context2;
private readonly HttpContextBase _context;
private IDictionary ContextItems
{
// create wrapper only once!
var wrapper = new HttpContextWrapper(context);
_context = () => wrapper;
//get { return _context1 != null ? _context1.Items : _context2().Items; }
get { return _context != null ? _context.Items : HttpContext.Current.Items; }
}
public HttpRequestCacheProvider(Func<HttpContextBase> context)
// for unit tests
public HttpRequestCacheProvider(HttpContextBase context)
{
_context = context;
}
// main constructor
// will use HttpContext.Current
public HttpRequestCacheProvider(/*Func<HttpContext> context*/)
{
//_context2 = context;
}
protected override IEnumerable<DictionaryEntry> GetDictionaryEntries()
{
const string prefix = CacheItemPrefix + "-";
return _context().Items.Cast<DictionaryEntry>()
return ContextItems.Cast<DictionaryEntry>()
.Where(x => x.Key is string && ((string)x.Key).StartsWith(prefix));
}
protected override void RemoveEntry(string key)
{
_context().Items.Remove(key);
ContextItems.Remove(key);
}
protected override object GetEntry(string key)
{
return _context().Items[key];
return ContextItems[key];
}
#region Lock
protected override IDisposable ReadLock
{
// there's no difference between ReadLock and WriteLock here
get { return WriteLock; }
}
protected override IDisposable WriteLock
{
// NOTE
// could think about just overriding base.Locker to return a different
// object but then we'd create a ReaderWriterLockSlim per request,
// which is less efficient than just using a basic monitor lock.
get
{
// FIXME
// we still need something to lock though
// some locker in the context items?
object locker;
lock (this)
{
locker = ContextItems["locker"] ?? (ContextItems["locker"] = new object());
}
return new MonitorLock(locker);
}
}
#endregion
#region Get
public override object GetCacheItem(string cacheKey, Func<object> getCacheItem)
@@ -50,15 +104,18 @@ namespace Umbraco.Core.Cache
Lazy<object> result;
using (var lck = new UpgradeableReadLock(Locker))
using (WriteLock)
{
result = _context().Items[cacheKey] as Lazy<object>; // null if key not found
if (result == null || (result.IsValueCreated && GetSafeLazyValue(result) == null)) // get exceptions as null
{
lck.UpgradeToWriteLock();
result = ContextItems[cacheKey] as Lazy<object>; // null if key not found
// cannot create value within the lock, so if result.IsValueCreated is false, just
// do nothing here - means that if creation throws, a race condition could cause
// more than one thread to reach the return statement below and throw - accepted.
if (result == null || GetSafeLazyValue(result, true) == null) // get non-created as NonCreatedValue & exceptions as null
{
result = new Lazy<object>(getCacheItem);
_context().Items[cacheKey] = result;
ContextItems[cacheKey] = result;
}
}

65
src/Umbraco.Core/Cache/HttpRuntimeCacheProvider.cs
View File

@@ -2,6 +2,7 @@
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Web.Caching;
using CacheItemPriority = System.Web.Caching.CacheItemPriority;
@@ -12,6 +13,11 @@ namespace Umbraco.Core.Cache
/// </summary>
internal class HttpRuntimeCacheProvider : DictionaryCacheProviderBase, IRuntimeCacheProvider
{
// locker object that supports upgradeable read locking
// does not need to support recursion if we implement the cache correctly and ensure
// that methods cannot be reentrant, ie we do NOT create values while holding a lock.
private readonly ReaderWriterLockSlim _locker = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion);
private readonly System.Web.Caching.Cache _cache;
public HttpRuntimeCacheProvider(System.Web.Caching.Cache cache)
@@ -36,6 +42,20 @@ namespace Umbraco.Core.Cache
return _cache.Get(key);
}
#region Lock
protected override IDisposable ReadLock
{
get { return new ReadLock(_locker); }
}
protected override IDisposable WriteLock
{
get { return new WriteLock(_locker); }
}
#endregion
#region Get
/// <summary>
@@ -72,26 +92,48 @@ namespace Umbraco.Core.Cache
// on the Lazy lock to ensure that getCacheItem runs once and everybody waits on it, while the global
// application lock has been released.
// Note that the Lazy execution may produce a null value.
// Must make sure (for backward compatibility) that we pretend they are not in the cache.
// So if we find an entry in the cache that already has its value created and is null,
// pretend it was not there. If value is not already created, wait... and return null, that's
// what prior code did.
// NOTE
// The Lazy value creation may produce a null value.
// Must make sure (for backward compatibility) that we pretend they are not in the cache.
// So if we find an entry in the cache that already has its value created and is null,
// pretend it was not there. If value is not already created, wait... and return null, that's
// what prior code did.
// NOTE
// The Lazy value creation may throw.
// So... the null value _will_ be in the cache but never returned
Lazy<object> result;
using (var lck = new UpgradeableReadLock(Locker))
// Fast!
// Only one thread can enter an UpgradeableReadLock at a time, but it does not prevent other
// threads to enter a ReadLock in the meantime -- only upgrading to WriteLock will prevent all
// reads. We first try with a normal ReadLock for maximum concurrency and take the penalty of
// having to re-lock in case there's no value. Would need to benchmark to figure out whether
// it's worth it, though...
using (new ReadLock(_locker))
{
result = _cache.Get(cacheKey) as Lazy<object>; // null if key not found
if (result == null || (result.IsValueCreated && GetSafeLazyValue(result) == null)) // get exceptions as null
{
lck.UpgradeToWriteLock();
}
var value = result == null ? null : GetSafeLazyValue(result);
if (value != null) return value;
using (var lck = new UpgradeableReadLock(_locker))
{
result = _cache.Get(cacheKey) as Lazy<object>; // null if key not found
// cannot create value within the lock, so if result.IsValueCreated is false, just
// do nothing here - means that if creation throws, a race condition could cause
// more than one thread to reach the return statement below and throw - accepted.
if (result == null || GetSafeLazyValue(result, true) == null) // get non-created as NonCreatedValue & exceptions as null
{
result = new Lazy<object>(getCacheItem);
var absolute = isSliding ? System.Web.Caching.Cache.NoAbsoluteExpiration : (timeout == null ? System.Web.Caching.Cache.NoAbsoluteExpiration : DateTime.Now.Add(timeout.Value));
var sliding = isSliding == false ? System.Web.Caching.Cache.NoSlidingExpiration : (timeout ?? System.Web.Caching.Cache.NoSlidingExpiration);
lck.UpgradeToWriteLock();
_cache.Insert(cacheKey, result, dependency, absolute, sliding, priority, removedCallback);
}
}
@@ -140,7 +182,10 @@ namespace Umbraco.Core.Cache
var absolute = isSliding ? System.Web.Caching.Cache.NoAbsoluteExpiration : (timeout == null ? System.Web.Caching.Cache.NoAbsoluteExpiration : DateTime.Now.Add(timeout.Value));
var sliding = isSliding == false ? System.Web.Caching.Cache.NoSlidingExpiration : (timeout ?? System.Web.Caching.Cache.NoSlidingExpiration);
_cache.Insert(cacheKey, result, dependency, absolute, sliding, priority, removedCallback);
using (new WriteLock(_locker))
{
_cache.Insert(cacheKey, result, dependency, absolute, sliding, priority, removedCallback);
}
}
public void InsertCacheItem(string cacheKey, Func<object> getCacheItem, TimeSpan? timeout = null, bool isSliding = false, CacheItemPriority priority = CacheItemPriority.Normal, CacheItemRemovedCallback removedCallback = null, string[] dependentFiles = null)

4
src/Umbraco.Core/CacheHelper.cs
View File

@@ -44,7 +44,7 @@ namespace Umbraco.Core
: this(
new HttpRuntimeCacheProvider(HttpRuntime.Cache),
new StaticCacheProvider(),
new HttpRequestCacheProvider(() => new HttpContextWrapper(HttpContext.Current)))
new HttpRequestCacheProvider())
{
}
@@ -56,7 +56,7 @@ namespace Umbraco.Core
: this(
new HttpRuntimeCacheProvider(cache),
new StaticCacheProvider(),
new HttpRequestCacheProvider(() => new HttpContextWrapper(HttpContext.Current)))
new HttpRequestCacheProvider())
{
}

32
src/Umbraco.Core/MonitorLock.cs Normal file
View File

@@ -0,0 +1,32 @@
using System;
namespace Umbraco.Core
{
/// <summary>
/// Provides an equivalent to the c# lock statement, to be used in a using block.
/// </summary>
/// <remarks>Ie replace <c>lock (o) {...}</c> by <c>using (new MonitorLock(o)) { ... }</c></remarks>
public class MonitorLock : IDisposable
{
private readonly object _locker;
private readonly bool _entered;
/// <summary>
/// Initializes a new instance of the <see cref="MonitorLock"/> class with an object to lock.
/// </summary>
/// <param name="locker">The object to lock.</param>
/// <remarks>Should always be used within a using block.</remarks>
public MonitorLock(object locker)
{
_locker = locker;
_entered = false;
System.Threading.Monitor.Enter(_locker, ref _entered);
}
void IDisposable.Dispose()
{
if (_entered)
System.Threading.Monitor.Exit(_locker);
}
}
}

1
src/Umbraco.Core/Umbraco.Core.csproj
View File

@@ -215,6 +215,7 @@
<Compile Include="Models\PublishedContent\PublishedContentModel.cs" />
<Compile Include="Models\PublishedContent\PublishedContentModelFactoryResolver.cs" />
<Compile Include="Models\TemplateNode.cs" />
<Compile Include="MonitorLock.cs" />
<Compile Include="Packaging\Models\InstallAction.cs" />
<Compile Include="Packaging\Models\InstallationSummary.cs" />
<Compile Include="Packaging\Models\MetaData.cs" />

2
src/Umbraco.Tests/Cache/HttpRequestCacheProviderTests.cs
View File

@@ -14,7 +14,7 @@ namespace Umbraco.Tests.Cache
{
base.Setup();
_ctx = new FakeHttpContextFactory("http://localhost/test");
_provider = new HttpRequestCacheProvider(() => _ctx.HttpContext);
_provider = new HttpRequestCacheProvider(_ctx.HttpContext);
}
internal override ICacheProvider Provider