-
Notifications
You must be signed in to change notification settings - Fork 107
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
Reduce allocations for generated constructors #1075
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Example of difference in generated code, taken from Code generated on master branch public sealed class DataReader : IDataReader, ...
{
private IntPtr ThisPtr => _inner == null ? (((IWinRTObject)this).NativeObject).ThisPtr : _inner.ThisPtr;
private IObjectReference _inner = null;
private readonly Dictionary<Type, object> _lazyInterfaces;
...
internal sealed class _IDataReaderFactory { ... }
public DataReader(IInputStream inputStream) : this(((Func<IObjectReference>)(() => {
IntPtr ptr = (_IDataReaderFactory.Instance.CreateDataReader(inputStream));
try
{
return (ComWrappersSupport.GetObjectReferenceForInterface(ptr));
}
finally
{
MarshalInspectable<object>.DisposeAbi(ptr);
}
}))())
{
ComWrappersSupport.RegisterObjectForInterface(this, ThisPtr);
ComWrappersHelper.Init(_inner, false);
}
...
internal DataReader(IObjectReference objRef)
{
_inner = objRef.As(GuidGenerator.GetIID(typeof(IDataReader).GetHelperType()));
_lazyInterfaces = new Dictionary<Type, object>(1)
{
{typeof(global::System.IDisposable), new Lazy<global::System.IDisposable>(() => (global::System.IDisposable)(object)new SingleInterfaceOptimizedObject(typeof(global::System.IDisposable), _inner ?? ((IWinRTObject)this).NativeObject))},
};
}
...
private global::System.IDisposable AsInternal(InterfaceTag<global::System.IDisposable> _) => ((Lazy<global::System.IDisposable>)_lazyInterfaces[typeof(global::System.IDisposable)]).Value;
} Code generated on topic branch jlarkin/objref-func public sealed class DataReader : IDataReader, ...
{
private IntPtr ThisPtr => _inner == null ? (((IWinRTObject)this).NativeObject).ThisPtr : _inner.ThisPtr;
private IObjectReference _inner = null;
private volatile global::System.IDisposable _lazy_global__System_IDisposable;
private global::System.IDisposable Make__lazy_global__System_IDisposable()
{
global::System.Threading.Interlocked.CompareExchange(ref _lazy_global__System_IDisposable, (global::System.IDisposable)(object)new SingleInterfaceOptimizedObject(typeof(global::System.IDisposable), _inner ?? ((IWinRTObject)this).NativeObject), null);
return _lazy_global__System_IDisposable;
}
...
internal sealed class _IDataReaderFactory { ... }
public DataReader(IInputStream inputStream)
{
IntPtr ptr = (_IDataReaderFactory.Instance.CreateDataReader(inputStream));
try
{
_inner = ComWrappersSupport.GetObjectReferenceForInterface(ptr);
}
finally
{
MarshalInspectable<object>.DisposeAbi(ptr);
}
ComWrappersSupport.RegisterObjectForInterface(this, ThisPtr);
ComWrappersHelper.Init(_inner, false);
}
...
internal DataReader(IObjectReference objRef)
{
_inner = objRef.As(GuidGenerator.GetIID(typeof(IDataReader).GetHelperType()));
}
...
private global::System.IDisposable AsInternal(InterfaceTag<global::System.IDisposable> _) => Make__lazy_global__System_IDisposable(); |
manodasanW
reviewed
Jan 12, 2022
manodasanW
reviewed
Jan 12, 2022
manodasanW
reviewed
Jan 18, 2022
manodasanW
reviewed
Jan 18, 2022
manodasanW
approved these changes
Jan 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For generated types, we often have a public constructor and then internal/base constructors. We reduce the space used when calling one of these public constructors by refactoring the relevant ObjectReference code. Instead of creating and immediately invoking a lambda to give us an object reference, we inline the code in the public constructor.
Also included is a refactoring of the generated "_lazyInterfaces" dictionary. We saw before that it was being initialized in those internal constructors, but without any references to local variables. We move them to the global level and use the Interlocked.CompareExchange pattern of lazy initialization, rather than the .NET Lazy type.
I have run some benchmarks and have two scenarios.
For the change to constructors and making/invoking a lambda, we only make this change for net5.0 code. The code to assign _inner is different for netstandard compatibility, making our optimization untenable. However, the change to the lazy interface initializations provides improvements to all runtimes (net5, net3, netstd2).
Net5.0
NetStandard2.0