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

Cache HasCustomAttribute #5269

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

metalgearsloth
Copy link
Contributor

Makes debug a lot faster. No idea if static semaphoreslims are bad but my game didn't explodeTM.

Resolves #5268

image

image

Makes debug a lot faster. No idea if static semaphoreslims are bad but my game didn't explodeTM.
Copy link
Member

@PJB3005 PJB3005 left a comment

Choose a reason for hiding this comment

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

It would be way better if usages of this function were cached instead, but I guess I'll allow it...


namespace Robust.Shared.Utility
{
public static class TypeHelpers
{
private static SemaphoreSlim _hasSlim = new(1);
private static Dictionary<MemberInfo, Dictionary<Type, bool>> _hasAttribute = new();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private static Dictionary<MemberInfo, Dictionary<Type, bool>> _hasAttribute = new();
private static readonly Dictionary<MemberInfo, Dictionary<Type, bool>> _hasAttribute = new();

@@ -183,7 +188,18 @@ public static bool IsBackingField(this MemberInfo memberInfo)

public static bool HasCustomAttribute<T>(this MemberInfo memberInfo) where T : Attribute
{
return memberInfo.GetCustomAttribute<T>() != null;
using (_hasSlim.WaitGuard())
Copy link
Member

Choose a reason for hiding this comment

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

Just use lock() on the dictionary instead.

@metalgearsloth
Copy link
Contributor Author

It would be way better if usages of this function were cached instead, but I guess I'll allow it...

If you want me to do it I can do it, it's primarily dirty and addcomp calls afaik.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HasCustomAttribute is really expensive in debug
2 participants