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

JIT: Make JIT time constants from calls to Type methods and properties #4920

Closed
omariom opened this issue Jan 9, 2016 · 39 comments
Closed
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions optimization tenet-performance Performance related issue
Milestone

Comments

@omariom
Copy link
Contributor

omariom commented Jan 9, 2016

If JIT could treat the calls like typeof(T).IsValueType, typeof(IInterface).IsAssingnableFrom(typeof(T)) etc. as JIT time constants that could allow us to write more performant generic code.

category:cq
theme:type-intrinsics
skill-level:intermediate
cost:large

@mikedn
Copy link
Contributor

mikedn commented Jan 9, 2016

Hmm, what would be the use of typeof(IInterface).IsAssingnableFrom(typeof(T) in generic code?

Test if a value type implement a certain interface? But if it does you'll probably need to cast to that interface anyway.

@omariom
Copy link
Contributor Author

omariom commented Jan 9, 2016

@mikedn
For example, we could use the trick EqualityComparer uses to dispatch handling to proper methods without casting and boxing.
Like this:

if (typeof(IInterface1).IsAssigneableFrom(typeof(T)))
{
      DispatcherForIInterface1.Instance.Dispatch(value);
}
else if (typeof(IInterface2).IsAssigneableFrom(typeof(T)))
{
    DispatcherForIInterface2.Instance.Dispatch(value);
}
else
{
     throw new Exception("Bad type.");
}

@mikedn
Copy link
Contributor

mikedn commented Jan 9, 2016

I see. With this type of optimizations the JIT could reduce EqualityComparer<int>.CreateComparer() to just return new GenericEqualityComparer<int>();. And similar code size reductions could also be obtained for nullable and enum types. Even for reference types some size reductions could be obtained if the JIT knows that reference types cannot be enums nor nullables.

@mikedn
Copy link
Contributor

mikedn commented Jan 9, 2016

While we're at it I think we can also add the case of

if (typeof(T) == typeof(int)) {...

The if is eliminated as expected when T is a value type other than int but it is not eliminated when T is a reference type (obviously, if T is a reference type it can never be int).

@benaadams
Copy link
Member

👍

@benaadams
Copy link
Member

Although for coreclr you need to do

typeof(T).GetTypeInfo().IsValueType
typeof(IInterface).GetTypeInfo().IsAssignableFrom(typeof(T).GetTypeInfo())

@omariom
Copy link
Contributor Author

omariom commented Jan 9, 2016

Nice property is that the optimizations won't disappear when Ngen'ed or .NetNative'ed, will they?

@mikedn
Copy link
Contributor

mikedn commented Jan 9, 2016

Although for coreclr you need to do...

Yep, GetTypeInfo might complicate things a bit but hopefully not make it impossible. For typeof the return is likely always a RuntimeType.

Nice property is that the optimizations won't disappear when Ngen'ed or .NetNative'ed, will they?

At least the most useful one should stay. typeof(int).IsValueType must always return true but perhaps for typeof(SomeThirdPartyValueType).IsValueType runtime evaluation would be needed in case the type is changed to a reference type (though such a change would probably invalidate the native image).

@mikedn
Copy link
Contributor

mikedn commented Jan 9, 2016

Maybe a table with various type methods and properties would be useful to look at. It's not exhaustive (I skipped many things that are unlikely to be useful or just too complex).

Member Notes
BaseType typeof(int).BaseType.IsValueType is always false for example. Unlikely to be very useful
GetEnumUnderlyingType Could be useful to implement an efficient Enum.HasFlag<T>
GetTypeCode Type.GetTypeCode(typeof(int)) is always TypeCode.Int32. Could be useful to replace a bunch of ifs with a switch.
IsAbstract typeof(int).IsAbstract is always false. Doesn't seem useful
IsArray typeof(int).IsArray is always false. Doesn't seem useful
IsAssignableFrom Already discussed
IsClass typeof(int).IsClass is always false. Similar to IsValueType but I'd say IsValueType is more common
IsEnum typeof(SomeEnumType).IsEnum is always true. Seen in the CreateComparer method mentioned previously
IsPrimitive typeof(int).IsPrimitive is always true. Might be useful sometimes
IsSealed typeof(int).IsSealed is always true. Doesn't seem useful
IsSubclassOf Somewhat similar to IsAssignableFrom but seems far less useful
IsValueType Already mentioned

@omariom
Copy link
Contributor Author

omariom commented Jan 9, 2016

And.. cough.. typeof(T).ContainsReferences if it happens to be implemented 😉

@mikedn
Copy link
Contributor

mikedn commented Jan 9, 2016

And.. cough.. typeof(T).ContainsReferences if it happen to be implemented 😉

Indeed 😄

And there's another thing that's not directly related to Type properties:

        static void Test<T>(T x)
        {
            if (x is int)
            {
                Console.WriteLine("sure it is");
            }
        }

For T = int the type check should be a no-op but in reality it does boxing. AFAIR that's what you were trying to avoid in corefxlab's AppendUntyped PR.

@omariom
Copy link
Contributor Author

omariom commented Jan 9, 2016

It is even more readable than typeof(T) == typeof(int).

@omariom
Copy link
Contributor Author

omariom commented Jan 16, 2016

@mikedn
To sum up that scenario.

In these cases, given value is of generic type T and the code is value type instantiated, JIT could call the methods without boxing, inlining them if appropriate.

// 1. Currently boxes value on each comparison and casting to object.
if (value is IInterface)
{
     ((IInterface)(object)value).Method();
}
else if (value  is IAnotherInterface)
{
      ((IAnotherInterface)(object)value).Method2();
}

// 2. Variation of the  previous one.
if (typeof(IInterface).IsAssingnableFrom(typeof(T))
{
     ((IInterface)(object)value).Method();
}
else typeof(IAnotherInterface).IsAssingnableFrom(typeof(T)
{
      ((IAnotherInterface)(object)value).Method2();
}

Here JIT could avoid boxing as well. Currently it boxes once per comparison.

if (value is int)
{
     int intValue = (int)(object)value);
}
else if (value  is long)
{
      long longValue = ((long)(object)value);
}

@mikedn
Copy link
Contributor

mikedn commented Jan 16, 2016

JIT could call the methods without boxing, inlining them if appropriate.

Are you talking about calls to Method and Method2? Yes, boxing could be avoided there but note that the JIT would need to make a copy of the value before calling. That may limit the usefulness of such code.

@omariom
Copy link
Contributor Author

omariom commented Jan 16, 2016

Because boxing implies making a copy?
Even if so not boxing and inlining save much more.
And for primitives copying is no op, isn't it?

@mikedn
Copy link
Contributor

mikedn commented Jan 16, 2016

Because boxing implies making a copy?

Yep.

Even if so not boxing and inlining save much more.

Yep. The point is that in some cases (methods that change the value) that kind of code will not do what you want. There's nothing wrong with the optimization itself.

And for primitives copying is no op, isn't it?

Depends on the method. If the method is inlined then the copy might get eliminated. If the method is not inlined then the compiler won't know that the method doesn't mutate the struct so it needs to make a copy.

@redknightlois
Copy link

+1 on this... making generic numerical libraries even if you need to use T4 scripts to write the code and the dispatching code would be doable with this optimization.

@mikedn
Copy link
Contributor

mikedn commented Feb 13, 2016

+1 on this... making generic numerical libraries even if you need to use T4 scripts to write the code and the dispatching code would be doable with this optimization.

Some stuff that would help such libraries already works (though not to well but that's a separate issue). I think it would be useful if you could provide some examples that add value to the optimizations that are discussed here.

@redknightlois
Copy link

Sure, I had to dig a very old code (pre CoreCLR) and it looks like this. Eventually I just moved into a different direction; but mostly because performance was not good.

https://github.com/redknightlois/cudalearn/blob/8e3ffb58d75ec593397ec1a43de59b6fc0548a88/CudaLearn/Matrix%601.cs#L336

@mikedn
Copy link
Contributor

mikedn commented Feb 13, 2016

Hmm, I haven't tested your code but it's likely that it already works as expected. The presence of the T epsilon parameter forces the JIT to generate different code for Equals(Matrix<float>, Matrix<float>, float>) and Equals(Matrix<double>, Matrix<double>, double>). As a result, comparisons like typeof(T) == typeof(float)) become constants and they and the associated dead code are eliminated.

@redknightlois
Copy link

Worth taking a look into that. But there are many cases where there is no T epsilon parameters.

Like:
https://github.com/redknightlois/cudalearn/blob/8e3ffb58d75ec593397ec1a43de59b6fc0548a88/CudaLearn/Matrix%601.cs#L366

Also there is the (int)(object)value and (T)(object)MatrixHelper.Determinant(t) coercing like:
https://github.com/redknightlois/cudalearn/blob/8e3ffb58d75ec593397ec1a43de59b6fc0548a88/CudaLearn/Matrix%601.cs#L395

@omariom
Copy link
Contributor Author

omariom commented Feb 13, 2016

@redknightlois

             if (typeof(T) == typeof(float)) 
             { 
                 var t = m as Matrix<float>; 
                 return (T)(object)MatrixHelper.Determinant(t); 
             } 
             else if (typeof(T) == typeof(double)) 
             { 
                 var t = m as Matrix<double>; 
                 return (T)(object)MatrixHelper.Determinant(t); 
             } 

Luckily exactly for such cases JIT can avoid boxing in (T)(object).

@redknightlois
Copy link

Nice!! This was some old code I ended up rewriting differently to avoid, among other things, the boxing and dispatch cost. Definitely it is worth to retest this on RyuJIT and see how it fares. I found more than a few cases lately where RyuJIT just blows the LegacyJIT out of the water.

@mikedn
Copy link
Contributor

mikedn commented Feb 13, 2016

But there are many cases where there is no T epsilon parameters.

That likely works as well. Even if there's no T parameter that method is a member of Matrix<T> and a T may be used somewhere in the method (and actually it is used). As a result the JIT has little choice but to generate an unshared instantiation and that's what makes expressions such as typeof(T) == typeof(float) constant.

@omariom
Copy link
Contributor Author

omariom commented Feb 21, 2016

One more constant when T is a value type. For reference types it is not because of array covariance:

bool Test<T>(T[] array)
{
    return array.GetType() == typeof(T[]);
}

@mikedn
Copy link
Contributor

mikedn commented Feb 21, 2016

For reference types it is not because of array covariance:

Array covariance has nothing to do with this. Like with all other similar cases that involve reference types this check isn't constant because the same generated code is used for all Ts.

@omariom
Copy link
Contributor Author

omariom commented Feb 21, 2016

@mikedn
Yes, you are right.

@jamesqo
Copy link
Contributor

jamesqo commented Jul 22, 2016

+1'd this. Since it's the only way to tell if a generic type is byref/value/etc since all the generic stuff is specialized at runtime, having this optimized would be very useful and help out a lot of apps.

@GSPP
Copy link

GSPP commented Jul 22, 2016

@jamesqo how do you like var isRefType = (object)default(T) == null? :) Not sure if this is optimized away, though.

@jamesqo
Copy link
Contributor

jamesqo commented Jul 22, 2016

@GSPP Unfortunately, that will not work for nullables. bool IsNull<T>() => default(T) == null; IsNull<int?>() is true.

@redknightlois
Copy link

redknightlois commented Jun 7, 2018

An actual use case of this:

    public interface IAllocatorOptions
    {
    }

    public interface IAllocationHandler<TAllocator> where TAllocator : struct, IAllocator<TAllocator>, IAllocator, IDisposable
    {
        void OnAllocate(ref TAllocator allocator, BlockPointer ptr);
        void OnRelease(ref TAllocator allocator, BlockPointer ptr);
    }

    public interface ILifecycleHandler<TAllocator> where TAllocator : struct, IAllocator<TAllocator>, IAllocator, IDisposable
    {
        void BeforeInitialize(ref TAllocator allocator);
        void AfterInitialize(ref TAllocator allocator);
        void BeforeDispose(ref TAllocator allocator);
        void BeforeFinalization(ref TAllocator allocator);
    }

    public interface ILowMemoryHandler<TAllocator> where TAllocator : struct, IAllocator<TAllocator>, IAllocator, IDisposable
    {
        void NotifyLowMemory(ref TAllocator allocator);
        void NotifyLowMemoryOver(ref TAllocator allocator);
    }

    public interface IRenewable<TAllocator> where TAllocator : struct, IAllocator<TAllocator>, IAllocator, IDisposable
    {
        void Renew(ref TAllocator allocator);
    }

    public interface IAllocator { }

    public unsafe interface IAllocator<T> where T : struct, IAllocator, IDisposable
    {
        int Allocated { get; }

        void Initialize(ref T allocator);

        void Configure<TConfig>(ref T allocator, ref TConfig configuration)
            where TConfig : struct, IAllocatorOptions;

        void Allocate(ref T allocator, int size, out BlockPointer.Header* header);
        void Release(ref T allocator, in BlockPointer.Header* header);
        void Reset(ref T allocator);
    }

    public sealed class Allocator<TAllocator> : IDisposable, ILowMemoryHandler
        where TAllocator : struct, IAllocator<TAllocator>, IAllocator, IDisposable
    {
        private TAllocator _allocator;
        private readonly SingleUseFlag _disposeFlag = new SingleUseFlag();

        ~Allocator()
        {
            if (_allocator is ILifecycleHandler<TAllocator>)
                ((ILifecycleHandler<TAllocator>)_allocator).BeforeFinalization(ref _allocator);

            Dispose();
        }

        public void Initialize<TBlockAllocatorOptions>(TBlockAllocatorOptions options)
            where TBlockAllocatorOptions : struct, IAllocatorOptions
        {
            if (_allocator is ILifecycleHandler<TAllocator>)
                ((ILifecycleHandler<TAllocator>)_allocator).BeforeInitialize(ref _allocator);

            _allocator.Initialize(ref _allocator);
            _allocator.Configure(ref _allocator, ref options);

            if (_allocator is ILifecycleHandler<TAllocator>)
                ((ILifecycleHandler<TAllocator>)_allocator).AfterInitialize(ref _allocator);
        }

        public int Allocated
        {
            [MethodImpl(MethodImplOptions.AggressiveInlining)]
            get { return _allocator.Allocated; }
        }
        
        public BlockPointer Allocate(int size)
        {
            unsafe
            {
                _allocator.Allocate(ref _allocator, size, out var header);

                var ptr = new BlockPointer(header);
                if (typeof(IAllocationHandler<TAllocator>).IsAssignableFrom(_allocator.GetType()))
                    ((IAllocationHandler<TAllocator>)_allocator).OnAllocate(ref _allocator, ptr);

                return ptr;
            }
        }

        public BlockPointer<TType> Allocate<TType>(int size) where TType : struct
        {
            unsafe
            {
                _allocator.Allocate(ref _allocator, size * Unsafe.SizeOf<TType>(), out var header);

                var ptr = new BlockPointer(header);
                if (typeof(IAllocationHandler<TAllocator>).IsAssignableFrom(_allocator.GetType()))
                    ((IAllocationHandler<TAllocator>)_allocator).OnAllocate(ref _allocator, ptr);

                return new BlockPointer<TType>(ptr);
            }
        }

        public void Release(BlockPointer ptr)
        {
            unsafe
            {
                if (typeof(IAllocationHandler<TAllocator>).IsAssignableFrom(_allocator.GetType()))
                    ((IAllocationHandler<TAllocator>)_allocator).OnRelease(ref _allocator, ptr);

                _allocator.Release(ref _allocator, in ptr._header);
            }
        }

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public void Renew()
        {
            if (typeof(IRenewable<TAllocator>).IsAssignableFrom(_allocator.GetType()))
                ((IRenewable<TAllocator>)_allocator).Renew(ref _allocator);
            else
                throw new NotSupportedException($".{nameof(Renew)}() is not supported for this allocator type.");
        }

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public void Reset()
        {
            _allocator.Reset(ref _allocator);
        }
        
        public void Dispose()
        {
            if (_disposeFlag.Raise())
                _allocator.Dispose();

            GC.SuppressFinalize(this);
        }

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public void LowMemory()
        {
            if (_allocator is ILowMemoryHandler<TAllocator>)
                ((ILowMemoryHandler<TAllocator>)_allocator).NotifyLowMemory(ref _allocator);
        }

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public void LowMemoryOver()
        {
            if (_allocator is ILowMemoryHandler<TAllocator>)
                ((ILowMemoryHandler<TAllocator>)_allocator).NotifyLowMemoryOver(ref _allocator);
        }
    }

The IAllocationHandler<TAllocator> and IRenewable<TAllocator> are conditional interfaces... if you support them then they should generate the appropriate code for them. Right now that optimization doesnt happen which means it is very costly.

EDIT: As a side note, it seems that code like:

 if (_allocator is IRenewable<TAllocator> a)
     a.Renew()

Does work even though the interface is conditional. The code generated is better, but still requires a runtime check.

cc @AndyAyersMS

@tannergooding
Copy link
Member

Are the Architecture/Runtime checks currently treated as JIT constants?

That is:

  • Environment.Is64BitOperatingSystem
  • Environment.Is64BitOperatingSystemWhen32BitProcess
  • Environment.Is64BitProcess
  • RuntimeInformation.IsOSPlatform
  • RuntimeInformation.OSArchitecture
  • RuntimeInformation.ProcessArchitecture
  • etc

@AndyAyersMS
Copy link
Member

I don't believe they are handled in any special way.

@tannergooding
Copy link
Member

@AndyAyersMS, it might be nice if we had a way to tell the JIT (maybe internal only): This method returns a constant

It would then be possible to place that on various methods (like the above) and could be hooked up to the ValNumStore for constant folding purposes, without needing to wire up special handling for each method/property.

@jkotas
Copy link
Member

jkotas commented Jun 7, 2018

The way to do that is to make the property just return a readonly static field. I believe that the JIT has logic that treats readonly static fields as constants.

A problem with that is cctor triggering - this only works once the constructor has run. Tiered JIT solves this nicely.

@redknightlois
Copy link

@jkotas as far as my experiments go with 2.1 (non-tiered JIT, didn't try that yet but on the horizon to do so) that is not really the case.

@benaadams
Copy link
Member

as far as my experiments go with 2.1 (non-tiered JIT, didn't try that yet but on the horizon to do so) that is not really the case.

All mine show it to be the case.

Don't have a static .cctor, so the class is marked as beforefieldinit; otherwise the check will always be embedded.

The first access to a static field (not method); will cause all of the static readonly fields to become Jit consts. The method that makes that access however will have the checks in (i.e. not be consts for that method) - so you don't want it to be your hot path method. This is where the tiered Jit resolves it as that hot path with then be compiled a second time (as is a hot path); and the second time they will all be consts.

Also a readonly static in a generic type will not be treated this way (as each type of the generic has its own static so there's extra lookup complexity) so move it out to a non-generic type for the effect.

HTH

@redknightlois
Copy link

redknightlois commented Jun 7, 2018

Ohhh you mean for primitive types. Yes, for those it works. It aint general though.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 30, 2020
@msftgits msftgits added this to the Future milestone Jan 30, 2020
@omariom omariom changed the title RyuJIT: [betternes] Make JIT time constants from calls to Type methods and props RyuJIT: [betternes] Make JIT time constants from calls to Type methods and properties May 6, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@BruceForstall BruceForstall removed the JitUntriaged CLR JIT issues needing additional triage label Nov 25, 2020
@BruceForstall BruceForstall changed the title RyuJIT: [betternes] Make JIT time constants from calls to Type methods and properties JIT: Make JIT time constants from calls to Type methods and properties Nov 25, 2020
@EgorBo
Copy link
Member

EgorBo commented Oct 11, 2023

I am closing this since IsValueType and IsAssingnableFrom are intrinsified long time ago, the rest can be enabled on demand (actually, I even had a branch where some of them were folded naturally without intrinsification - #92562 (comment))

@EgorBo EgorBo closed this as completed Oct 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Nov 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions optimization tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests