Improve handling of Exception<T>#272
Conversation
niemyjski
left a comment
There was a problem hiding this comment.
Thanks for the PR, can we also add test cases specifically addressed in the issue around structs. I also left some feedback about performance concerns
@ejsmith should we be worried about this creating lots of new stacks due to hash changes for type names?
|
How about this? private static readonly ConcurrentDictionary<Type, string> TypeNameCache = new ConcurrentDictionary<Type, string>();
public static string GetRealTypeName(this Type t) {
if (TypeNameCache.TryGetValue(t, out string name)) {
return name;
}
if (!t.IsGenericType)
return t.FullName.Replace('+','.');
StringBuilder sb = new StringBuilder();
ReadOnlySpan<char> fullName = t.FullName.AsSpan();
int plusIndex = fullName.IndexOf('+');
if (plusIndex > 0) {
sb.Append(fullName.Slice(0, plusIndex).ToArray());
sb.Append('.');
}
int length = fullName.IndexOf('`') - (plusIndex > 0 ? plusIndex + 1 : 0);
sb.Append(fullName.Slice(plusIndex > 0 ? plusIndex + 1 : 0, length).ToArray());
sb.Append('<');
bool appendComma = false;
foreach (Type arg in t.GetGenericArguments()) {
if (appendComma) { sb.Append(','); }
sb.Append(GetRealTypeName(arg));
appendComma = true;
}
sb.Append('>');
return TypeNameCache.GetOrAdd(t, sb.ToString());
} |
|
Should I just use the TypeNameHelper? Exceptionless.Net/src/Exceptionless/Demystifier/TypeNameHelper.cs Lines 48 to 53 in 118b006 Only issue I see is that it includes the If this is okay, I will use that. I also suggest we add a cache of some kind to |
Added TypeNameHelper.GetTypeFullDisplayName and cache
|
@niemyjski How is that? Do you want a couple more test cases? |
|
@ejsmith any chance of a quick review? |
niemyjski
left a comment
There was a problem hiding this comment.
Left some comments, It's nice that we found an existing method that handles this!
All sorted. Thanks for the review! |
|
Thanks for the PR! These are really nice changes @elachlan was there any other changes you wanted to get in prior to a release? |
I won't have the time to work on this for the next two weeks. I was going to look at using spans in place of string operations at some point. |
|
I think if anything, we just report this possible optimization to the upstream dependency. |
|
Is this the upstream? |
|
Yes |
Fixes #83.
Reference: https://stackoverflow.com/questions/17480990/get-name-of-generic-class-without-tilde