jason-roberts / FeatureToggle

Simple, reliable feature toggles in .NET
http://dontcodetired.com/blog/?tag=/featuretoggle
Apache License 2.0
689 stars 111 forks source link

Performance - Replace use of Activator in Is<T> #158

Closed rcollette closed 6 years ago

rcollette commented 6 years ago

Activator is notably slow.
Since you already have put a zero arg constructor constraint on T, you should be able to just do

return new T().FeatureEnabled;

If for some reason that doesn't work then you can look at a generic version of DynamicMethod, but I don't think you will need it. https://stackoverflow.com/a/22651558

mikanyg commented 6 years ago

As far as I can see, then current implementation does what you suggest https://github.com/jason-roberts/FeatureToggle/blob/master/src/FeatureToggle.Common/Is.cs

rcollette commented 6 years ago

That's interesting. When I decompile in Jetbrains Rider I get a different set of source. I wonder if new T() actually compiles to using Activator.

// Decompiled with JetBrains decompiler
// Type: FeatureToggle.Is`1
// Assembly: FeatureToggle.Common, Version=4.0.2.0, Culture=neutral, PublicKeyToken=null
// MVID: 56948869-D82D-4DE1-93B1-08121C96F0D7
// Assembly location: /Users/richardcollette/.nuget/packages/featuretoggle.common/4.0.2/lib/netstandard1.4/FeatureToggle.Common.dll

using System;

namespace FeatureToggle
{
  public static class Is<T> where T : IFeatureToggle, new()
  {
    public static bool Enabled
    {
      get
      {
        return Activator.CreateInstance<T>().FeatureEnabled;
      }
    }

    public static bool Disabled
    {
      get
      {
        return !Activator.CreateInstance<T>().FeatureEnabled;
      }
    }
  }
}
mikanyg commented 6 years ago

Decompiling is not nearly as good as looking at actual source code. That is my experience when decompiling .NET framework BCL code and comparing with the open source code.

jason-roberts commented 6 years ago

Is this something that needs to be looked at in v5 or can we close @rcollette @mikanyg ?

mikanyg commented 6 years ago

I don’t know any other way to instantiate generic types than what is already implemented. So I would say close this issue.

rcollette commented 6 years ago

Close please.