Skip to content

Commit

Permalink
PR feedback updates
Browse files Browse the repository at this point in the history
- Improved doc comments for new overloads on ISharingLifetimeScope
- Split instance caching into two ConcurrentDictionary collections for single and "qualified" instances, to keep non-decorator paths from seeing performance impact
  • Loading branch information
VonOgre committed May 10, 2020
1 parent 4a476e2 commit 48f857d
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 12 deletions.
12 changes: 9 additions & 3 deletions src/Autofac/Core/ISharingLifetimeScope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,11 @@ public interface ISharingLifetimeScope : ILifetimeScope
/// possible secondary qualifying GUID key.
/// </summary>
/// <param name="primaryId">Key to look up.</param>
/// <param name="qualifyingId">Secondary key to look up.</param>
/// <param name="value">The instance that has the specified key.</param>
/// <param name="qualifyingId">
/// Secondary key to look up, to better identify an instance that wraps around another instance
/// or is otherwise "namespaced" by it.
/// </param>
/// <param name="value">The instance that has the specified keys.</param>
/// <returns><c>true</c> if the key was found; otherwise, <c>false</c>.</returns>
bool TryGetSharedInstance(Guid primaryId, Guid? qualifyingId, out object value);

Expand All @@ -73,7 +76,10 @@ public interface ISharingLifetimeScope : ILifetimeScope
/// possible secondary qualifying GUID key.
/// </summary>
/// <param name="primaryId">Key.</param>
/// <param name="qualifyingId">Secondary key.</param>
/// <param name="qualifyingId">
/// Secondary key, to better identify an instance that wraps around another instance
/// or is otherwise "namespaced" by it.
/// </param>
/// <param name="creator">A function that will create the instance when called.</param>
/// <returns>The shared instance.</returns>
object CreateSharedInstance(Guid primaryId, Guid? qualifyingId, Func<object> creator);
Expand Down
40 changes: 31 additions & 9 deletions src/Autofac/Core/Lifetime/LifetimeScope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public class LifetimeScope : Disposable, ISharingLifetimeScope, IServiceProvider
/// Protects shared instances from concurrent access. Other members and the base class are threadsafe.
/// </summary>
private readonly object _synchRoot = new object();
private readonly ConcurrentDictionary<(Guid, Guid?), object> _sharedInstances = new ConcurrentDictionary<(Guid, Guid?), object>();
private readonly ConcurrentDictionary<Guid, object> _sharedInstances = new ConcurrentDictionary<Guid, object>();
private readonly ConcurrentDictionary<(Guid, Guid), object> _sharedQualifiedInstances = new ConcurrentDictionary<(Guid, Guid), object>();
private object? _anonymousTag;
private LifetimeScope? parentScope;

Expand Down Expand Up @@ -82,7 +83,7 @@ protected LifetimeScope(IComponentRegistry componentRegistry, LifetimeScope pare
/// <param name="componentRegistry">Components used in the scope.</param>
public LifetimeScope(IComponentRegistry componentRegistry, object tag)
{
_sharedInstances[(SelfRegistrationId, null)] = this;
_sharedInstances[SelfRegistrationId] = this;
ComponentRegistry = componentRegistry ?? throw new ArgumentNullException(nameof(componentRegistry));
Tag = tag ?? throw new ArgumentNullException(nameof(tag));
RootLifetimeScope = this;
Expand Down Expand Up @@ -292,35 +293,56 @@ public object ResolveComponent(ResolveRequest request)
/// <inheritdoc />
public object CreateSharedInstance(Guid id, Func<object> creator)
{
return CreateSharedInstance(id, null, creator);
if (creator == null) throw new ArgumentNullException(nameof(creator));
lock (_synchRoot)
{
if (_sharedInstances.TryGetValue(id, out var result)) return result;

result = creator();
if (_sharedInstances.ContainsKey(id))
throw new DependencyResolutionException(string.Format(CultureInfo.CurrentCulture, LifetimeScopeResources.SelfConstructingDependencyDetected, result.GetType().FullName));

_sharedInstances.TryAdd(id, result);

return result;
}
}

/// <inheritdoc/>
public object CreateSharedInstance(Guid primaryId, Guid? qualifyingId, Func<object> creator)
{
if (creator == null) throw new ArgumentNullException(nameof(creator));
if (qualifyingId == null)
{
return CreateSharedInstance(primaryId, creator);
}

lock (_synchRoot)
{
var instanceKey = (primaryId, qualifyingId);
var instanceKey = (primaryId, qualifyingId.Value);

if (_sharedInstances.TryGetValue(instanceKey, out var result)) return result;
if (_sharedQualifiedInstances.TryGetValue(instanceKey, out var result)) return result;

result = creator();
if (_sharedInstances.ContainsKey(instanceKey))
if (_sharedQualifiedInstances.ContainsKey(instanceKey))
throw new DependencyResolutionException(string.Format(CultureInfo.CurrentCulture, LifetimeScopeResources.SelfConstructingDependencyDetected, result.GetType().FullName));

_sharedInstances.TryAdd(instanceKey, result);
_sharedQualifiedInstances.TryAdd(instanceKey, result);

return result;
}
}

/// <inheritdoc />
public bool TryGetSharedInstance(Guid id, out object value) => TryGetSharedInstance(id, null, out value);
public bool TryGetSharedInstance(Guid id, out object value) => _sharedInstances.TryGetValue(id, out value);

/// <inheritdoc/>
public bool TryGetSharedInstance(Guid primaryId, Guid? qualifyingId, out object value) => _sharedInstances.TryGetValue((primaryId, qualifyingId), out value);
public bool TryGetSharedInstance(Guid primaryId, Guid? qualifyingId, out object value)
{
return qualifyingId == null
? TryGetSharedInstance(primaryId, out value)
: _sharedQualifiedInstances.TryGetValue((primaryId, qualifyingId.Value), out value);
}

/// <summary>
/// Gets the disposer associated with this container. Instances can be associated
Expand Down

0 comments on commit 48f857d

Please sign in to comment.