-
Notifications
You must be signed in to change notification settings - Fork 447
Use ActivatorUtlities.CreateFactory instead of CreateInstance #1643
Conversation
- Turns out CreateFactory is much much faster - Added a benchmark for hub activation
@@ -9,6 +9,8 @@ namespace Microsoft.AspNetCore.SignalR | |||
{ | |||
public class DefaultHubActivator<THub> : IHubActivator<THub> where THub: Hub | |||
{ | |||
// Object factory for all this Hub type | |||
private static ObjectFactory _objectFactory = ActivatorUtilities.CreateFactory(typeof(THub), Type.EmptyTypes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a pretty heavy operation to have in a static constructor. Also if the hub type is registered with the service provider then we'll never even use the factory. IMO create the factory when it is needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at that, I can make it lazy.
🆙 📅 . Seems like it got even faster after it was made lazy 😄 |
@@ -9,6 +9,8 @@ namespace Microsoft.AspNetCore.SignalR | |||
{ | |||
public class DefaultHubActivator<THub> : IHubActivator<THub> where THub: Hub | |||
{ | |||
// Object factory for THub instances | |||
private static Lazy<ObjectFactory> _objectFactory = new Lazy<ObjectFactory>(() => ActivatorUtilities.CreateFactory(typeof(THub), Type.EmptyTypes)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readonly
Why did you need full on |
You're right. I guess worst case we would have compiled a few times. Meh. |
Before
After
Lazy
Fixes #1570