Skip to content

Added caching to type->shortname and shortname->type conversions. #30

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

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open
Changes from 1 commit
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
32 changes: 19 additions & 13 deletions Hyperion/Extensions/TypeEx.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ namespace Hyperion.Extensions
{
public static class TypeEx
{
private static ConcurrentDictionary<string, Type> _shortNameToTypeCache = new ConcurrentDictionary<string, Type>();
private static ConcurrentDictionary<Type, string> _typeToShortNameCache = new ConcurrentDictionary<Type, string>();

//Why not inline typeof you ask?
//Because it actually generates calls to get the type.
//We prefetch all primitives here
Expand Down Expand Up @@ -200,24 +203,27 @@ public static int GetTypeSize(this Type type)

public static string GetShortAssemblyQualifiedName(this Type type)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not requirement, but an idea: GetShortAssemblyQualifiedName is actually used in 2 external places (TypeSerializer and ObjectSerializer). In both places this string is converted to a byte array immediately after. What we could do here, is to make this method return a byte array directly, avoiding string conversions in the future.

Copy link
Contributor Author

@nvivo nvivo Feb 9, 2017

Choose a reason for hiding this comment

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

@Horusiath I was looking into this. There is an issue with this optimization. For ObjectSerializer this idea works fine, but TypeSerializer has it's own way to serialize values directy to the stream. I tried replacing the StringSerializer for the ByteArraySerializer and the tests didn't pass. We would need 2 caches one for each format, or change something else to make them equal.

Another thing is that since GetShortAssemblyQualifiedName is recursive, caching itself helps a little resolving names the first time. Then again, we could add second cache, but increase memory usage. Not sure if there would be significant gains.

Using the cache should make the protocol at least as fast as it was before. Worst case, a dictionary + lock could be faster in some scenarios.

This is getting complicated... =) I believe it's better to merge this as it is and try to optimize this later.

{
string fullName;

if (type.IsGenericType)
{
var args = type.GetGenericArguments().Select(t => "[" + GetShortAssemblyQualifiedName(t) + "]");
fullName = type.Namespace + "." + type.Name + "[" + String.Join(",", args) + "]";
}
else
return _typeToShortNameCache.GetOrAdd(type, t =>
{
fullName = type.FullName;
}

return fullName + ", " + type.Assembly.GetName().Name;
string fullName;

if (t.IsGenericType)
{
var args = t.GetGenericArguments().Select(gt => "[" + GetShortAssemblyQualifiedName(gt) + "]");
fullName = t.Namespace + "." + t.Name + "[" + String.Join(",", args) + "]";
}
else
{
fullName = t.FullName;
}

return fullName + ", " + t.Assembly.GetName().Name;
});
}

public static Type GetTypeFromShortName(string shortName)
{
return Type.GetType(shortName, ShortNameAssemblyResolver, null, true);
return _shortNameToTypeCache.GetOrAdd(shortName, name => Type.GetType(shortName, ShortNameAssemblyResolver, null, true));
}

private static Assembly ShortNameAssemblyResolver(AssemblyName name)
Expand Down