Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve performance of RegistrationKey #41

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions BoDi.Performance.Tests/Benchmarks/ResolveFromGenericType.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
using BenchmarkDotNet.Attributes;

namespace BODi.Performance.Tests.Benchmarks
{
public class ResolveFromGenericType : SingleContainerBenchmarkBase
{
[Benchmark(Baseline = true, Description = "v1.4")]
public object Version_1_4()
{
return Container14.Resolve<IGenericRegistered<int>>();
}

[Benchmark(Description = "v1.BoDi_Concurrent_Dictionary_And_Lazy")]
public object Version_1_BoDi_Concurrent_Dictionary_And_Lazy()
{
return Container1Concurrent_Dictionary_And_Lazy.Resolve<IGenericRegistered<int>>();
}

[Benchmark(Description = "Current")]
public object CurrentVersion()
{
return ContainerCurrent.Resolve<IGenericRegistered<int>>();
}
}
}
15 changes: 11 additions & 4 deletions BoDi.Performance.Tests/Benchmarks/SingleContainerBenchmarkBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,21 @@ namespace BODi.Performance.Tests.Benchmarks
{
[HtmlExporter]
[MarkdownExporterAttribute.GitHub]
[MemoryDiagnoser]
[MinColumn, MaxColumn, MeanColumn, MedianColumn, RankColumn]
[Orderer(SummaryOrderPolicy.FastestToSlowest, MethodOrderPolicy.Declared)]
public abstract class SingleContainerBenchmarkBase
{
protected internal IObjectContainer ContainerCurrent;
protected internal BoDi1_4.IObjectContainer Container14;
protected internal BoDi_Concurrent_Dictionary_And_Lazy.IObjectContainer Container1Concurrent_Dictionary_And_Lazy;
protected internal ObjectContainer ContainerCurrent;
protected internal BoDi1_4.ObjectContainer Container14;
protected internal BoDi_Concurrent_Dictionary_And_Lazy.ObjectContainer Container1Concurrent_Dictionary_And_Lazy;

[GlobalSetup]
public void Setup()
{
Container14 = new BoDi1_4.ObjectContainer();
Container14.RegisterFactoryAs(_ => new FactoryRegistered());
Container14.RegisterTypeAs(typeof(GenericRegistered<>), typeof(IGenericRegistered<>));
Container14.RegisterTypeAs<TypeRegistered, TypeRegistered>();
Container14.RegisterTypeAs<AllRegistered1, IAllRegisteredFromType>();
Container14.RegisterTypeAs<AllRegistered2, IAllRegisteredFromType>();
Expand All @@ -32,6 +34,7 @@ public void Setup()

Container1Concurrent_Dictionary_And_Lazy = new BoDi_Concurrent_Dictionary_And_Lazy.ObjectContainer();
Container1Concurrent_Dictionary_And_Lazy.RegisterFactoryAs(_ => new FactoryRegistered());
Container1Concurrent_Dictionary_And_Lazy.RegisterTypeAs(typeof(GenericRegistered<>), typeof(IGenericRegistered<>));
Container1Concurrent_Dictionary_And_Lazy.RegisterTypeAs<TypeRegistered, TypeRegistered>();
Container1Concurrent_Dictionary_And_Lazy.RegisterTypeAs<AllRegistered1, IAllRegisteredFromType>();
Container1Concurrent_Dictionary_And_Lazy.RegisterTypeAs<AllRegistered2, IAllRegisteredFromType>();
Expand All @@ -44,6 +47,7 @@ public void Setup()

ContainerCurrent = new ObjectContainer();
ContainerCurrent.RegisterFactoryAs(_ => new FactoryRegistered());
ContainerCurrent.RegisterTypeAs(typeof(GenericRegistered<>), typeof(IGenericRegistered<>));
ContainerCurrent.RegisterTypeAs<TypeRegistered, TypeRegistered>();
ContainerCurrent.RegisterTypeAs<AllRegistered1, IAllRegisteredFromType>();
ContainerCurrent.RegisterTypeAs<AllRegistered2, IAllRegisteredFromType>();
Expand All @@ -53,9 +57,12 @@ public void Setup()
ContainerCurrent.RegisterFactoryAs<IAllRegisteredFromFactory>(_ => new AllRegistered2());
ContainerCurrent.RegisterFactoryAs<IAllRegisteredFromFactory>(_ => new AllRegistered3());
ContainerCurrent.RegisterFactoryAs<IAllRegisteredFromFactory>(_ => new AllRegistered4());

}

protected internal interface IGenericRegistered<T> { }

protected internal class GenericRegistered<T> : IGenericRegistered<T> { }

protected internal class FactoryRegistered { }

protected internal class TypeRegistered { }
Expand Down
29 changes: 8 additions & 21 deletions BoDi/BoDi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -291,29 +291,19 @@ public override string ToString()
}
}

private struct RegistrationKey
private readonly struct RegistrationKey : IEquatable<RegistrationKey>
{
public readonly Type Type;
private readonly Type typeGroup;
public readonly string Name;

public RegistrationKey(Type type, string name)
{
if (type == null) throw new ArgumentNullException("type");

Type = type;
Type = type ?? throw new ArgumentNullException(nameof(type));
typeGroup = (type.IsGenericType && !type.IsGenericTypeDefinition) ? type.GetGenericTypeDefinition() : type;
Name = name;
}

private Type TypeGroup
{
get
{
if (Type.IsGenericType && !Type.IsGenericTypeDefinition)
return Type.GetGenericTypeDefinition();
return Type;
}
}

public override string ToString()
{
Debug.Assert(Type.FullName != null);
Expand All @@ -323,10 +313,10 @@ public override string ToString()
return string.Format("{0}('{1}')", Type.FullName, Name);
}

bool Equals(RegistrationKey other)
public bool Equals(RegistrationKey other)
{
var isInvertable = other.TypeGroup == Type || other.Type == TypeGroup || other.Type == Type;
return isInvertable && String.Equals(other.Name, Name, StringComparison.CurrentCultureIgnoreCase);
var isInvertable = other.Type == Type || other.typeGroup == Type || other.Type == typeGroup;
return isInvertable && other.Name == Name;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh forgot this is still in. Need to check the impact first

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From ResolveFromType
| Current | 226.3 ns | 1.15 ns | 1.07 ns | 224.2 ns | 227.8 ns | 226.2 ns | 0.0577 | - | - | 272 B |
=> ~2.5% difference

Is the CurrentCultureIgnoreCase required here? What is the intention for it? Wouldn't you always resolve with the exact same key as it was registered?

}

public override bool Equals(object obj)
Expand All @@ -338,10 +328,7 @@ public override bool Equals(object obj)

public override int GetHashCode()
{
unchecked
{
return TypeGroup.GetHashCode();
}
return typeGroup.GetHashCode();
}
}

Expand Down