From 528e0e9804a961c8d2d0f1fcb0a46c5b22cb60f6 Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Tue, 17 Jan 2017 19:08:01 +1100 Subject: [PATCH 01/16] Alternative System.Lazy implementation --- src/mscorlib/src/System/Lazy.cs | 447 ++++++++++++++++++-------------- 1 file changed, 246 insertions(+), 201 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index 85cbb9816ad0..fe5b3b9fcdc1 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -27,14 +27,6 @@ namespace System { - // Lazy is generic, but not all of its state needs to be generic. Avoid creating duplicate - // objects per instantiation by putting them here. - internal static class LazyHelpers - { - // Dummy object used as the value of m_threadSafeObj if in PublicationOnly mode. - internal static readonly object PUBLICATION_ONLY_SENTINEL = new object(); - } - /// /// Provides support for lazy initialization. /// @@ -50,63 +42,22 @@ internal static class LazyHelpers [ComVisible(false)] [DebuggerTypeProxy(typeof(System_LazyDebugView<>))] [DebuggerDisplay("ThreadSafetyMode={Mode}, IsValueCreated={IsValueCreated}, IsValueFaulted={IsValueFaulted}, Value={ValueForDebugDisplay}")] - public class Lazy + public class Lazy : + ISerializable { - -#region Inner classes - /// - /// wrapper class to box the initialized value, this is mainly created to avoid boxing/unboxing the value each time the value is called in case T is - /// a value type - /// - [Serializable] - class Boxed - { - internal Boxed(T value) - { - m_value = value; - } - internal T m_value; - } - - - /// - /// Wrapper class to wrap the excpetion thrown by the value factory - /// - class LazyInternalExceptionHolder + private interface ILazyItem { - internal ExceptionDispatchInfo m_edi; - internal LazyInternalExceptionHolder(Exception ex) - { - m_edi = ExceptionDispatchInfo.Capture(ex); - } + T Value { get; } + bool IsValueCreated { get; } + bool IsValueFaulted { get; } + LazyThreadSafetyMode Mode { get; } } -#endregion - - // A dummy delegate used as a : - // 1- Flag to avoid recursive call to Value in None and ExecutionAndPublication modes in m_valueFactory - // 2- Flag to m_threadSafeObj if ExecutionAndPublication mode and the value is known to be initialized - static readonly Func ALREADY_INVOKED_SENTINEL = delegate - { - Debug.Assert(false, "ALREADY_INVOKED_SENTINEL should never be invoked."); - return default(T); - }; - - //null --> value is not created - //m_value is Boxed --> the value is created, and m_value holds the value - //m_value is LazyExceptionHolder --> it holds an exception - private object m_boxed; - - // The factory delegate that returns the value. - // In None and ExecutionAndPublication modes, this will be set to ALREADY_INVOKED_SENTINEL as a flag to avoid recursive calls - [NonSerialized] - private Func m_valueFactory; - // null if it is not thread safe mode - // LazyHelpers.PUBLICATION_ONLY_SENTINEL if PublicationOnly mode - // object if ExecutionAndPublication mode (may be ALREADY_INVOKED_SENTINEL if the value is already initialized) - [NonSerialized] - private object m_threadSafeObj; + // m_implementation, a volatile reference, is set to null after m_value has been set + private volatile ILazyItem m_implementation; + // m_value eventually stores the lazily created value. It is ready when m_implementation = null. + private T m_value; /// /// Initializes a new instance of the class that @@ -116,7 +67,7 @@ internal LazyInternalExceptionHolder(Exception ex) /// An instance created with this constructor may be used concurrently from multiple threads. /// public Lazy() - : this(LazyThreadSafetyMode.ExecutionAndPublication) + : this(CreateInstance.Factory, LazyThreadSafetyMode.ExecutionAndPublication) { } @@ -130,7 +81,8 @@ public Lazy() /// public Lazy(T value) { - m_boxed = new Boxed(value); + m_value = value; + m_implementation = null; } /// @@ -157,8 +109,8 @@ public Lazy(Func valueFactory) /// /// true if this instance should be usable by multiple threads concurrently; false if the instance will only be used by one thread at a time. /// - public Lazy(bool isThreadSafe) : - this(isThreadSafe? LazyThreadSafetyMode.ExecutionAndPublication : LazyThreadSafetyMode.None) + public Lazy(bool isThreadSafe) : + this(CreateInstance.Factory, GetMode(isThreadSafe)) { } @@ -168,12 +120,11 @@ public Lazy(bool isThreadSafe) : /// /// The lazy thread-safety mode mode /// mode contains an invalid valuee - public Lazy(LazyThreadSafetyMode mode) + public Lazy(LazyThreadSafetyMode mode) : + this(CreateInstance.Factory, mode) { - m_threadSafeObj = GetObjectFromMode(mode); } - /// /// Initializes a new instance of the class /// that uses a specified initialization function and a specified thread-safety mode. @@ -185,8 +136,8 @@ public Lazy(LazyThreadSafetyMode mode) /// /// is /// a null reference (Nothing in Visual Basic). - public Lazy(Func valueFactory, bool isThreadSafe) - : this(valueFactory, isThreadSafe ? LazyThreadSafetyMode.ExecutionAndPublication : LazyThreadSafetyMode.None) + public Lazy(Func valueFactory, bool isThreadSafe) : + this(valueFactory, GetMode(isThreadSafe)) { } @@ -206,25 +157,140 @@ public Lazy(Func valueFactory, LazyThreadSafetyMode mode) if (valueFactory == null) throw new ArgumentNullException(nameof(valueFactory)); - m_threadSafeObj = GetObjectFromMode(mode); - m_valueFactory = valueFactory; + m_implementation = CreateInitializerFromMode(mode, this, valueFactory); } +#region constructor helpers + + private static class CreateInstance + { + private static T construct() + { + try + { + return (T)(Activator.CreateInstance(typeof(T))); + } + catch (MissingMethodException) + { + throw new MissingMemberException(Environment.GetResourceString("Lazy_CreateValue_NoParameterlessCtorForT")); + } + } + + public readonly static Func Factory = construct; + } + + private static LazyThreadSafetyMode GetMode(bool isThreadSafe) + { + return isThreadSafe ? LazyThreadSafetyMode.ExecutionAndPublication : LazyThreadSafetyMode.None; + } + + private static ILazyItem CreateInitializerFromMode(LazyThreadSafetyMode mode, Lazy owner, Func valueFactory) + { + switch (mode) + { + case LazyThreadSafetyMode.None: return new None(owner, valueFactory); + case LazyThreadSafetyMode.PublicationOnly: return new PublicationOnly(owner, valueFactory); + case LazyThreadSafetyMode.ExecutionAndPublication: return new ExecutionAndPublication(owner, valueFactory); + + default: + throw new ArgumentOutOfRangeException(nameof(mode), Environment.GetResourceString("Lazy_ctor_ModeInvalid")); + } + } + +#endregion + /// - /// Static helper function that returns an object based on the given mode. it also throws an exception if the mode is invalid + /// Creates the underlying value using the factory object, placing the result inside this + /// object, and then used the Lazy objects ILazyItem implemenation.to refer to it. /// - private static object GetObjectFromMode(LazyThreadSafetyMode mode) + /// The object factory used to create the underlying object + /// true when called with ExecutionAndPublication, false + /// when called with None + /// The underlying object + private T CreateValue(Func factory, LazyThreadSafetyMode mode) { - if (mode == LazyThreadSafetyMode.ExecutionAndPublication) - return new object(); - else if (mode == LazyThreadSafetyMode.PublicationOnly) - return LazyHelpers.PUBLICATION_ONLY_SENTINEL; - else if (mode != LazyThreadSafetyMode.None) - throw new ArgumentOutOfRangeException(nameof(mode), Environment.GetResourceString("Lazy_ctor_ModeInvalid")); - - return null; // None mode + try + { + m_value = factory(); + m_implementation = null; + return m_value; + } + catch (Exception exception) when (!ReferenceEquals(CreateInstance.Factory, factory)) + { + m_implementation = new LazyException(exception, mode); + throw; + } } + /// + /// Creates the underlying value using the factory object into a helper object and, for the + /// first object to complete its factory, uses that objects ILazyItem implementation + /// + /// The object factory used to create the underlying object + /// The publication object, used for synchronisation and comparison + /// The underlying object + private T CreateValuePublicationOnly(Func factory, PublicationOnly comparand) + { + var possibleValue = factory(); + lock (comparand) + { + if (ReferenceEquals(m_implementation, comparand)) + { + m_value = possibleValue; + m_implementation = null; + } + } + return m_value; + } + + /// + /// Entry point to underlying object creation used by the ExecutionAndPublication method. + /// This should only be called from that location, and should be under a lock. + /// + /// The object factory used to create the underlying object + /// Used to determine if we still need to create the underlying object + /// + private T CreateValueExecutionAndPublication(Func factory, ExecutionAndPublication comparand) + { + // it's possible for multiple calls to have piled up behind the lock, so we need to check + // to see if the ExecutionAndPublication object is still the current implementation. + return ReferenceEquals(m_implementation, comparand) ? CreateValue(factory, LazyThreadSafetyMode.ExecutionAndPublication) : Value; + } + +#region Serialization + // to remain compatible with previous version, custom serialization has been added + // which should be binary compatible. Only valid values were ever serialized. Exceptions + // were thrown from the serializer, which halted it, if the Lazy object through an exception. + + /// + /// wrapper class to box the initialized value, this is mainly created to avoid boxing/unboxing the value each time the value is called in case T is + /// a value type + /// + [Serializable] + class Boxed + { + internal Boxed(T value) + { + m_value = value; + } + internal T m_value; + } + + public Lazy(SerializationInfo information, StreamingContext context) + { + var boxed = (Boxed)information.GetValue("m_boxed", typeof(Boxed)); + m_value = boxed.m_value; + m_implementation = null; + } + + void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context) + { + var boxed = new Boxed(Value); + info.AddValue("m_boxed", boxed); + } + + #endregion + /// Forces initialization during serialization. /// The StreamingContext for the serialization operation. [OnSerializing] @@ -254,7 +320,7 @@ internal T ValueForDebugDisplay { return default(T); } - return ((Boxed)m_boxed).m_value; + return Value; } } @@ -265,9 +331,10 @@ internal LazyThreadSafetyMode Mode { get { - if (m_threadSafeObj == null) return LazyThreadSafetyMode.None; - if (m_threadSafeObj == (object)LazyHelpers.PUBLICATION_ONLY_SENTINEL) return LazyThreadSafetyMode.PublicationOnly; - return LazyThreadSafetyMode.ExecutionAndPublication; + var implementation = m_implementation; + if (implementation == null) + return LazyThreadSafetyMode.None; // we don't know the mode anymore + return implementation.Mode; } } @@ -276,7 +343,13 @@ internal LazyThreadSafetyMode Mode /// internal bool IsValueFaulted { - get { return m_boxed is LazyInternalExceptionHolder; } + get + { + var implementation = m_implementation; + if (implementation == null) + return false; + return implementation.IsValueFaulted; + } } /// Gets a value indicating whether the has been initialized. @@ -292,7 +365,10 @@ public bool IsValueCreated { get { - return m_boxed != null && m_boxed is Boxed; + var implementation = m_implementation; + if (implementation == null) + return true; + return implementation.IsValueCreated; } } @@ -322,147 +398,116 @@ public T Value { get { - Boxed boxed = null; - if (m_boxed != null ) - { - // Do a quick check up front for the fast path. - boxed = m_boxed as Boxed; - if (boxed != null) - { - return boxed.m_value; - } - - LazyInternalExceptionHolder exc = m_boxed as LazyInternalExceptionHolder; - Debug.Assert(exc != null); - exc.m_edi.Throw(); - } - - // Fall through to the slow path. - return LazyInitValue(); + var implementation = m_implementation; + if (implementation == null) + return m_value; + return implementation.Value; } } - /// - /// local helper method to initialize the value - /// - /// The inititialized T value - private T LazyInitValue() + #region ILazyItem implementations + + private sealed class LazyException : ILazyItem { - Boxed boxed = null; - LazyThreadSafetyMode mode = Mode; - if (mode == LazyThreadSafetyMode.None) + private readonly System.Runtime.ExceptionServices.ExceptionDispatchInfo m_exceptionInfo; + private readonly LazyThreadSafetyMode m_mode; + + internal LazyException(Exception exception, LazyThreadSafetyMode mode) { - boxed = CreateValue(); - m_boxed = boxed; + m_exceptionInfo = System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(exception); + m_mode = mode; } - else if (mode == LazyThreadSafetyMode.PublicationOnly) + + public bool IsValueCreated => false; + public bool IsValueFaulted => true; + public LazyThreadSafetyMode Mode => m_mode; + + public T Value { - boxed = CreateValue(); - if (boxed == null || - Interlocked.CompareExchange(ref m_boxed, boxed, null) != null) - { - // If CreateValue returns null, it means another thread successfully invoked the value factory - // and stored the result, so we should just take what was stored. If CreateValue returns non-null - // but another thread set the value we should just take what was stored. - boxed = (Boxed)m_boxed; - } - else + get { m_exceptionInfo.Throw(); return default(T); } + } + } + + abstract private class LazyInitializer : ILazyItem + { + protected Lazy Owner { get; } + protected Func Factory { get; private set; } + + protected Func TakeFactory() + { + var factory = Factory; + if (!ReferenceEquals(CreateInstance.Factory, factory)) { - // We successfully created and stored the value. At this point, the value factory delegate is - // no longer needed, and we don't want to hold onto its resources. - m_valueFactory = ALREADY_INVOKED_SENTINEL; + if (factory == null) + throw new InvalidOperationException(Environment.GetResourceString("Lazy_Value_RecursiveCallsToValue")); + Factory = null; } + return factory; } - else + + internal LazyInitializer(Lazy owner, Func factory) { - object threadSafeObj = Volatile.Read(ref m_threadSafeObj); - bool lockTaken = false; - try - { - if (threadSafeObj != (object)ALREADY_INVOKED_SENTINEL) - Monitor.Enter(threadSafeObj, ref lockTaken); - else - Debug.Assert(m_boxed != null); + Owner = owner; + Factory = factory; + } - if (m_boxed == null) - { - boxed = CreateValue(); - m_boxed = boxed; - Volatile.Write(ref m_threadSafeObj, ALREADY_INVOKED_SENTINEL); - } - else // got the lock but the value is not null anymore, check if it is created by another thread or faulted and throw if so - { - boxed = m_boxed as Boxed; - if (boxed == null) // it is not Boxed, so it is a LazyInternalExceptionHolder - { - LazyInternalExceptionHolder exHolder = m_boxed as LazyInternalExceptionHolder; - Debug.Assert(exHolder != null); - exHolder.m_edi.Throw(); - } - } - } - finally - { - if (lockTaken) - Monitor.Exit(threadSafeObj); - } + public bool IsValueCreated => false; + public bool IsValueFaulted => false; + abstract public T Value { get; } + abstract public LazyThreadSafetyMode Mode { get; } + } + + private sealed class None : LazyInitializer + { + internal None(Lazy owner, Func factory) : base(owner, factory) { } + + public override T Value + { + get { return Owner.CreateValue(TakeFactory(), LazyThreadSafetyMode.None); } + } + + public override LazyThreadSafetyMode Mode + { + get { return LazyThreadSafetyMode.None; } } - Debug.Assert(boxed != null); - return boxed.m_value; } - /// Creates an instance of T using m_valueFactory in case its not null or use reflection to create a new T() - /// An instance of Boxed. - private Boxed CreateValue() + private sealed class ExecutionAndPublication : LazyInitializer { - Boxed boxed = null; - LazyThreadSafetyMode mode = Mode; - if (m_valueFactory != null) + internal ExecutionAndPublication(Lazy owner, Func factory) : base(owner, factory) { } + + public override T Value { - try + get { - // check for recursion - if (mode != LazyThreadSafetyMode.PublicationOnly && m_valueFactory == ALREADY_INVOKED_SENTINEL) - throw new InvalidOperationException(Environment.GetResourceString("Lazy_Value_RecursiveCallsToValue")); - - Func factory = m_valueFactory; - if (mode != LazyThreadSafetyMode.PublicationOnly) // only detect recursion on None and ExecutionAndPublication modes - { - m_valueFactory = ALREADY_INVOKED_SENTINEL; - } - else if (factory == ALREADY_INVOKED_SENTINEL) + lock (this) // we're safe to lock on "this" as object is an private object used by Lazy { - // Another thread raced to successfully invoke the factory. - return null; + return Owner.CreateValueExecutionAndPublication(TakeFactory(), this); } - boxed = new Boxed(factory()); - } - catch (Exception ex) - { - if (mode != LazyThreadSafetyMode.PublicationOnly) // don't cache the exception for PublicationOnly mode - m_boxed = new LazyInternalExceptionHolder(ex); - throw; } } - else + + public override LazyThreadSafetyMode Mode { - try - { - boxed = new Boxed((T)Activator.CreateInstance(typeof(T))); + get { return LazyThreadSafetyMode.ExecutionAndPublication; } + } + } - } - catch (System.MissingMethodException) - { - Exception ex = new System.MissingMemberException(Environment.GetResourceString("Lazy_CreateValue_NoParameterlessCtorForT")); - if (mode != LazyThreadSafetyMode.PublicationOnly) // don't cache the exception for PublicationOnly mode - m_boxed = new LazyInternalExceptionHolder(ex); - throw ex; - } + private sealed class PublicationOnly : LazyInitializer + { + internal PublicationOnly(Lazy owner, Func factory) : base(owner, factory) { } + + public override T Value + { + get { return Owner.CreateValuePublicationOnly(Factory, this); } } - return boxed; + public override LazyThreadSafetyMode Mode + { + get { return LazyThreadSafetyMode.PublicationOnly; } + } } - + #endregion } /// A debugger view of the Lazy<T> to surface additional debugging properties and From e88a0a5191b56664b30b0c3559ac6fb0f78eb0d7 Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Wed, 18 Jan 2017 19:19:01 +1100 Subject: [PATCH 02/16] Minor naming fixes --- src/mscorlib/src/System/Lazy.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index fe5b3b9fcdc1..6466c12cf20e 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -164,7 +164,7 @@ public Lazy(Func valueFactory, LazyThreadSafetyMode mode) private static class CreateInstance { - private static T construct() + private static T Construct() { try { @@ -176,7 +176,7 @@ private static T construct() } } - public readonly static Func Factory = construct; + public readonly static Func Factory = Construct; } private static LazyThreadSafetyMode GetMode(bool isThreadSafe) @@ -204,8 +204,7 @@ private static ILazyItem CreateInitializerFromMode(LazyThreadSafetyMode mode, La /// object, and then used the Lazy objects ILazyItem implemenation.to refer to it. /// /// The object factory used to create the underlying object - /// true when called with ExecutionAndPublication, false - /// when called with None + /// The mode of the Lazy object /// The underlying object private T CreateValue(Func factory, LazyThreadSafetyMode mode) { From c69b791c174b23ceccc4f63ccbc92141d66386e2 Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Wed, 18 Jan 2017 19:45:48 +1100 Subject: [PATCH 03/16] Remove use of lock for PublicationOnly This involves a little bit of trickery. In avoiding the lock we need to go from m_implementation as PublicationOnly to null in a single step. We can't do this though without another object, so we reuse out Lazy object, which basically just spins its wheels until the value is ready. This shouldn't be a very common scenerio (i.e. hitting that code path) but it is possible. --- src/mscorlib/src/System/Lazy.cs | 105 +++++++++++++++++++++++++------- 1 file changed, 82 insertions(+), 23 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index 6466c12cf20e..e160e0f094ac 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -27,6 +27,18 @@ namespace System { + /// + /// ILazyItem<T> is used to determine the initialization logic that the Lazy object uses. + /// + /// + internal interface ILazyItem + { + T Value { get; } + bool IsValueCreated { get; } + bool IsValueFaulted { get; } + LazyThreadSafetyMode Mode { get; } + } + /// /// Provides support for lazy initialization. /// @@ -42,19 +54,12 @@ namespace System [ComVisible(false)] [DebuggerTypeProxy(typeof(System_LazyDebugView<>))] [DebuggerDisplay("ThreadSafetyMode={Mode}, IsValueCreated={IsValueCreated}, IsValueFaulted={IsValueFaulted}, Value={ValueForDebugDisplay}")] - public class Lazy : - ISerializable + public class Lazy + : ISerializable + , ILazyItem { - private interface ILazyItem - { - T Value { get; } - bool IsValueCreated { get; } - bool IsValueFaulted { get; } - LazyThreadSafetyMode Mode { get; } - } - // m_implementation, a volatile reference, is set to null after m_value has been set - private volatile ILazyItem m_implementation; + private volatile ILazyItem m_implementation; // m_value eventually stores the lazily created value. It is ready when m_implementation = null. private T m_value; @@ -184,7 +189,7 @@ private static LazyThreadSafetyMode GetMode(bool isThreadSafe) return isThreadSafe ? LazyThreadSafetyMode.ExecutionAndPublication : LazyThreadSafetyMode.None; } - private static ILazyItem CreateInitializerFromMode(LazyThreadSafetyMode mode, Lazy owner, Func valueFactory) + private static ILazyItem CreateInitializerFromMode(LazyThreadSafetyMode mode, Lazy owner, Func valueFactory) { switch (mode) { @@ -201,7 +206,7 @@ private static ILazyItem CreateInitializerFromMode(LazyThreadSafetyMode mode, La /// /// Creates the underlying value using the factory object, placing the result inside this - /// object, and then used the Lazy objects ILazyItem implemenation.to refer to it. + /// object, and then used the Lazy objects ILazyItem<T> implemenation.to refer to it. /// /// The object factory used to create the underlying object /// The mode of the Lazy object @@ -223,7 +228,7 @@ private T CreateValue(Func factory, LazyThreadSafetyMode mode) /// /// Creates the underlying value using the factory object into a helper object and, for the - /// first object to complete its factory, uses that objects ILazyItem implementation + /// first object to complete its factory, uses that objects ILazyItem<T> implementation /// /// The object factory used to create the underlying object /// The publication object, used for synchronisation and comparison @@ -231,15 +236,68 @@ private T CreateValue(Func factory, LazyThreadSafetyMode mode) private T CreateValuePublicationOnly(Func factory, PublicationOnly comparand) { var possibleValue = factory(); - lock (comparand) + + try {} + finally { - if (ReferenceEquals(m_implementation, comparand)) + // we run this in a finally block to ensure that we don't get a partial completion due + // to a Thread.Abort, which could mean that other threads might be left in infinite loops + var previous = Interlocked.CompareExchange(ref m_implementation, this, comparand); + if (previous == comparand) { m_value = possibleValue; m_implementation = null; } } - return m_value; + + return Value; + } + + private void WaitForPublicationOnly() + { + while (!ReferenceEquals(m_implementation, null)) + { + // CreateValuePublicationOnly temporarily sets m_implementation to "this". The Lazy implementation + // has an explicit iplementation of ILazyItem which just waits for the m_value to be set, which is + // signalled by m_implemenation then being set to null. + Thread.Sleep(0); + } + } + + T ILazyItem.Value + { + get + { + WaitForPublicationOnly(); + return Value; + } + } + + bool ILazyItem.IsValueCreated + { + get + { + WaitForPublicationOnly(); + return IsValueCreated; + } + } + + bool ILazyItem.IsValueFaulted + { + get + { + WaitForPublicationOnly(); + return IsValueFaulted; + } + } + + LazyThreadSafetyMode ILazyItem.Mode + { + get + { + WaitForPublicationOnly(); + return Mode; + } } /// @@ -288,7 +346,7 @@ void ISerializable.GetObjectData(SerializationInfo info, StreamingContext contex info.AddValue("m_boxed", boxed); } - #endregion +#endregion /// Forces initialization during serialization. /// The StreamingContext for the serialization operation. @@ -404,9 +462,9 @@ public T Value } } - #region ILazyItem implementations +#region ILazyItem implementations - private sealed class LazyException : ILazyItem + private sealed class LazyException : ILazyItem { private readonly System.Runtime.ExceptionServices.ExceptionDispatchInfo m_exceptionInfo; private readonly LazyThreadSafetyMode m_mode; @@ -427,13 +485,15 @@ public T Value } } - abstract private class LazyInitializer : ILazyItem + abstract private class LazyInitializer : ILazyItem { protected Lazy Owner { get; } protected Func Factory { get; private set; } protected Func TakeFactory() { + // None and ExecutionAndPublication use TakeFactory to protect against re-enterency, + // signalling recursion. var factory = Factory; if (!ReferenceEquals(CreateInstance.Factory, factory)) { @@ -506,7 +566,7 @@ public override LazyThreadSafetyMode Mode get { return LazyThreadSafetyMode.PublicationOnly; } } } - #endregion +#endregion } /// A debugger view of the Lazy<T> to surface additional debugging properties and @@ -547,6 +607,5 @@ public bool IsValueFaulted { get { return m_lazy.IsValueFaulted; } } - } } From 38c739c56f677d09f1e7a9f84c5e3ec34ad985d2 Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Thu, 19 Jan 2017 16:47:45 +1100 Subject: [PATCH 04/16] Need to check m_implementation before TakeFactory Threading bug. TakeFactory() would crash on second call that was banked up behind the lock. --- src/mscorlib/src/System/Lazy.cs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index e160e0f094ac..de32b7bc53f4 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -300,20 +300,6 @@ LazyThreadSafetyMode ILazyItem.Mode } } - /// - /// Entry point to underlying object creation used by the ExecutionAndPublication method. - /// This should only be called from that location, and should be under a lock. - /// - /// The object factory used to create the underlying object - /// Used to determine if we still need to create the underlying object - /// - private T CreateValueExecutionAndPublication(Func factory, ExecutionAndPublication comparand) - { - // it's possible for multiple calls to have piled up behind the lock, so we need to check - // to see if the ExecutionAndPublication object is still the current implementation. - return ReferenceEquals(m_implementation, comparand) ? CreateValue(factory, LazyThreadSafetyMode.ExecutionAndPublication) : Value; - } - #region Serialization // to remain compatible with previous version, custom serialization has been added // which should be binary compatible. Only valid values were ever serialized. Exceptions @@ -541,7 +527,9 @@ public override T Value { lock (this) // we're safe to lock on "this" as object is an private object used by Lazy { - return Owner.CreateValueExecutionAndPublication(TakeFactory(), this); + // it's possible for multiple calls to have piled up behind the lock, so we need to check + // to see if the ExecutionAndPublication object is still the current implementation. + return ReferenceEquals(Owner.m_implementation, this) ? Owner.CreateValue(TakeFactory(), LazyThreadSafetyMode.ExecutionAndPublication) : Owner.Value; } } } From c7c9dc367789f998b19d7816625f712d8af15c51 Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Sat, 21 Jan 2017 19:41:42 +1100 Subject: [PATCH 05/16] Minimize additional object creation Storing factory in Lazy object, and passing Lazy as a parameter to ILazyItem functions. This means that None & PublicationOnly now need no creation of secondary object. --- src/mscorlib/src/System/Lazy.cs | 327 ++++++++++++++++---------------- 1 file changed, 163 insertions(+), 164 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index de32b7bc53f4..fe32bc606057 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -27,18 +27,6 @@ namespace System { - /// - /// ILazyItem<T> is used to determine the initialization logic that the Lazy object uses. - /// - /// - internal interface ILazyItem - { - T Value { get; } - bool IsValueCreated { get; } - bool IsValueFaulted { get; } - LazyThreadSafetyMode Mode { get; } - } - /// /// Provides support for lazy initialization. /// @@ -56,13 +44,28 @@ internal interface ILazyItem [DebuggerDisplay("ThreadSafetyMode={Mode}, IsValueCreated={IsValueCreated}, IsValueFaulted={IsValueFaulted}, Value={ValueForDebugDisplay}")] public class Lazy : ISerializable - , ILazyItem { + /// + /// ILazyItem<T> is used to determine the initialization logic that the Lazy object uses. + /// + /// + internal interface ILazyItem + { + T GetValue(Lazy owner); + bool GetIsValueCreated(Lazy owner); + bool GetIsValueFaulted(Lazy owner); + LazyThreadSafetyMode GetMode(Lazy owner); + } + // m_implementation, a volatile reference, is set to null after m_value has been set - private volatile ILazyItem m_implementation; + private volatile ILazyItem m_implementation; + + // we ensure that m_factory is allow garbage collector to clean up any + private Func m_factory; // m_value eventually stores the lazily created value. It is ready when m_implementation = null. - private T m_value; + private T m_value; + /// /// Initializes a new instance of the class that @@ -87,6 +90,7 @@ public Lazy() public Lazy(T value) { m_value = value; + m_factory = null; m_implementation = null; } @@ -162,7 +166,25 @@ public Lazy(Func valueFactory, LazyThreadSafetyMode mode) if (valueFactory == null) throw new ArgumentNullException(nameof(valueFactory)); - m_implementation = CreateInitializerFromMode(mode, this, valueFactory); + m_factory = valueFactory; + + switch (mode) + { + case LazyThreadSafetyMode.None: + m_implementation = None.Instance; + break; + + case LazyThreadSafetyMode.PublicationOnly: + m_implementation = PublicationOnly.Instance; + break; + + case LazyThreadSafetyMode.ExecutionAndPublication: + m_implementation = new ExecutionAndPublication(); + break; + + default: + throw new ArgumentOutOfRangeException(nameof(mode), Environment.GetResourceString("Lazy_ctor_ModeInvalid")); + } } #region constructor helpers @@ -181,7 +203,7 @@ private static T Construct() } } - public readonly static Func Factory = Construct; + internal readonly static Func Factory = Construct; } private static LazyThreadSafetyMode GetMode(bool isThreadSafe) @@ -189,21 +211,30 @@ private static LazyThreadSafetyMode GetMode(bool isThreadSafe) return isThreadSafe ? LazyThreadSafetyMode.ExecutionAndPublication : LazyThreadSafetyMode.None; } - private static ILazyItem CreateInitializerFromMode(LazyThreadSafetyMode mode, Lazy owner, Func valueFactory) +#endregion + + private sealed class LazyException : ILazyItem { - switch (mode) + private readonly System.Runtime.ExceptionServices.ExceptionDispatchInfo m_exceptionInfo; + private readonly LazyThreadSafetyMode m_mode; + + internal LazyException(Exception exception, LazyThreadSafetyMode mode) { - case LazyThreadSafetyMode.None: return new None(owner, valueFactory); - case LazyThreadSafetyMode.PublicationOnly: return new PublicationOnly(owner, valueFactory); - case LazyThreadSafetyMode.ExecutionAndPublication: return new ExecutionAndPublication(owner, valueFactory); + m_exceptionInfo = System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(exception); + m_mode = mode; + } - default: - throw new ArgumentOutOfRangeException(nameof(mode), Environment.GetResourceString("Lazy_ctor_ModeInvalid")); + bool ILazyItem.GetIsValueCreated(Lazy owner) { return false; } + bool ILazyItem.GetIsValueFaulted(Lazy owner) { return true; } + LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) { return m_mode; } + + T ILazyItem.GetValue(Lazy owner) + { + m_exceptionInfo.Throw(); + return default(T); } } -#endregion - /// /// Creates the underlying value using the factory object, placing the result inside this /// object, and then used the Lazy objects ILazyItem<T> implemenation.to refer to it. @@ -211,8 +242,17 @@ private static ILazyItem CreateInitializerFromMode(LazyThreadSafetyMode mode, /// The object factory used to create the underlying object /// The mode of the Lazy object /// The underlying object - private T CreateValue(Func factory, LazyThreadSafetyMode mode) + private T CreateValue(LazyThreadSafetyMode mode) { + var factory = m_factory; + + if (!ReferenceEquals(CreateInstance.Factory, factory)) + { + if (factory == null) + throw new InvalidOperationException(Environment.GetResourceString("Lazy_Value_RecursiveCallsToValue")); + m_factory = null; + } + try { m_value = factory(); @@ -226,6 +266,52 @@ private T CreateValue(Func factory, LazyThreadSafetyMode mode) } } + private sealed class None : ILazyItem + { + private None() { } + + internal static ILazyItem Instance = new None(); + + T ILazyItem.GetValue(Lazy owner) + { + return owner.CreateValue(LazyThreadSafetyMode.None); + } + + LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) + { + return LazyThreadSafetyMode.None; + } + + bool ILazyItem.GetIsValueCreated(Lazy owner) { return false; } + bool ILazyItem.GetIsValueFaulted(Lazy owner) { return false; } + } + + private T CreateValueExecutionAndPublication(ExecutionAndPublication sync) + { + lock (sync) // we're safe to lock on "this" as object is an private object used by Lazy + { + // it's possible for multiple calls to have piled up behind the lock, so we need to check + // to see if the ExecutionAndPublication object is still the current implementation. + return ReferenceEquals(m_implementation, sync) ? CreateValue(LazyThreadSafetyMode.ExecutionAndPublication) : Value; + } + } + + private sealed class ExecutionAndPublication : ILazyItem + { + T ILazyItem.GetValue(Lazy owner) + { + return owner.CreateValueExecutionAndPublication(this); + } + + LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) + { + return LazyThreadSafetyMode.ExecutionAndPublication; + } + + bool ILazyItem.GetIsValueCreated(Lazy owner) { return false; } + bool ILazyItem.GetIsValueFaulted(Lazy owner) { return false; } + } + /// /// Creates the underlying value using the factory object into a helper object and, for the /// first object to complete its factory, uses that objects ILazyItem<T> implementation @@ -233,8 +319,12 @@ private T CreateValue(Func factory, LazyThreadSafetyMode mode) /// The object factory used to create the underlying object /// The publication object, used for synchronisation and comparison /// The underlying object - private T CreateValuePublicationOnly(Func factory, PublicationOnly comparand) + private T CreateValuePublicationOnly() { + var factory = m_factory; + if (factory == null) + return PublicationOnlyWaiter.Instance.GetValue(this); + var possibleValue = factory(); try {} @@ -242,10 +332,11 @@ private T CreateValuePublicationOnly(Func factory, PublicationOnly comparand) { // we run this in a finally block to ensure that we don't get a partial completion due // to a Thread.Abort, which could mean that other threads might be left in infinite loops - var previous = Interlocked.CompareExchange(ref m_implementation, this, comparand); - if (previous == comparand) + var previous = Interlocked.CompareExchange(ref m_implementation, PublicationOnlyWaiter.Instance, PublicationOnly.Instance); + if (previous == PublicationOnly.Instance) { m_value = possibleValue; + m_factory = null; m_implementation = null; } } @@ -253,6 +344,26 @@ private T CreateValuePublicationOnly(Func factory, PublicationOnly comparand) return Value; } + private sealed class PublicationOnly : ILazyItem + { + private PublicationOnly() { } + + internal static ILazyItem Instance = new PublicationOnly(); + + T ILazyItem.GetValue(Lazy owner) + { + return owner.CreateValuePublicationOnly(); + } + + LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) + { + return LazyThreadSafetyMode.PublicationOnly; + } + + bool ILazyItem.GetIsValueCreated(Lazy owner) { return false; } + bool ILazyItem.GetIsValueFaulted(Lazy owner) { return false; } + } + private void WaitForPublicationOnly() { while (!ReferenceEquals(m_implementation, null)) @@ -264,39 +375,35 @@ private void WaitForPublicationOnly() } } - T ILazyItem.Value + class PublicationOnlyWaiter + : ILazyItem { - get + private PublicationOnlyWaiter() { } + + internal static ILazyItem Instance = new PublicationOnlyWaiter(); + + T ILazyItem.GetValue(Lazy owner) { - WaitForPublicationOnly(); - return Value; + owner.WaitForPublicationOnly(); + return owner.Value; } - } - bool ILazyItem.IsValueCreated - { - get + bool ILazyItem.GetIsValueCreated(Lazy owner) { - WaitForPublicationOnly(); - return IsValueCreated; + owner.WaitForPublicationOnly(); + return owner.IsValueCreated; } - } - bool ILazyItem.IsValueFaulted - { - get + bool ILazyItem.GetIsValueFaulted(Lazy owner) { - WaitForPublicationOnly(); - return IsValueFaulted; + owner.WaitForPublicationOnly(); + return owner.IsValueFaulted; } - } - LazyThreadSafetyMode ILazyItem.Mode - { - get + LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) { - WaitForPublicationOnly(); - return Mode; + owner.WaitForPublicationOnly(); + return owner.Mode; } } @@ -377,7 +484,7 @@ internal LazyThreadSafetyMode Mode var implementation = m_implementation; if (implementation == null) return LazyThreadSafetyMode.None; // we don't know the mode anymore - return implementation.Mode; + return implementation.GetMode(this); } } @@ -391,7 +498,7 @@ internal bool IsValueFaulted var implementation = m_implementation; if (implementation == null) return false; - return implementation.IsValueFaulted; + return implementation.GetIsValueFaulted(this); } } @@ -411,7 +518,7 @@ public bool IsValueCreated var implementation = m_implementation; if (implementation == null) return true; - return implementation.IsValueCreated; + return implementation.GetIsValueCreated(this); } } @@ -444,117 +551,9 @@ public T Value var implementation = m_implementation; if (implementation == null) return m_value; - return implementation.Value; - } - } - -#region ILazyItem implementations - - private sealed class LazyException : ILazyItem - { - private readonly System.Runtime.ExceptionServices.ExceptionDispatchInfo m_exceptionInfo; - private readonly LazyThreadSafetyMode m_mode; - - internal LazyException(Exception exception, LazyThreadSafetyMode mode) - { - m_exceptionInfo = System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(exception); - m_mode = mode; - } - - public bool IsValueCreated => false; - public bool IsValueFaulted => true; - public LazyThreadSafetyMode Mode => m_mode; - - public T Value - { - get { m_exceptionInfo.Throw(); return default(T); } - } - } - - abstract private class LazyInitializer : ILazyItem - { - protected Lazy Owner { get; } - protected Func Factory { get; private set; } - - protected Func TakeFactory() - { - // None and ExecutionAndPublication use TakeFactory to protect against re-enterency, - // signalling recursion. - var factory = Factory; - if (!ReferenceEquals(CreateInstance.Factory, factory)) - { - if (factory == null) - throw new InvalidOperationException(Environment.GetResourceString("Lazy_Value_RecursiveCallsToValue")); - Factory = null; - } - return factory; - } - - internal LazyInitializer(Lazy owner, Func factory) - { - Owner = owner; - Factory = factory; + return implementation.GetValue(this); } - - public bool IsValueCreated => false; - public bool IsValueFaulted => false; - abstract public T Value { get; } - abstract public LazyThreadSafetyMode Mode { get; } } - - private sealed class None : LazyInitializer - { - internal None(Lazy owner, Func factory) : base(owner, factory) { } - - public override T Value - { - get { return Owner.CreateValue(TakeFactory(), LazyThreadSafetyMode.None); } - } - - public override LazyThreadSafetyMode Mode - { - get { return LazyThreadSafetyMode.None; } - } - } - - private sealed class ExecutionAndPublication : LazyInitializer - { - internal ExecutionAndPublication(Lazy owner, Func factory) : base(owner, factory) { } - - public override T Value - { - get - { - lock (this) // we're safe to lock on "this" as object is an private object used by Lazy - { - // it's possible for multiple calls to have piled up behind the lock, so we need to check - // to see if the ExecutionAndPublication object is still the current implementation. - return ReferenceEquals(Owner.m_implementation, this) ? Owner.CreateValue(TakeFactory(), LazyThreadSafetyMode.ExecutionAndPublication) : Owner.Value; - } - } - } - - public override LazyThreadSafetyMode Mode - { - get { return LazyThreadSafetyMode.ExecutionAndPublication; } - } - } - - private sealed class PublicationOnly : LazyInitializer - { - internal PublicationOnly(Lazy owner, Func factory) : base(owner, factory) { } - - public override T Value - { - get { return Owner.CreateValuePublicationOnly(Factory, this); } - } - - public override LazyThreadSafetyMode Mode - { - get { return LazyThreadSafetyMode.PublicationOnly; } - } - } -#endregion } /// A debugger view of the Lazy<T> to surface additional debugging properties and From 8c855c111941b6d0c4f4f346adc508edbbd36f2d Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Sun, 22 Jan 2017 20:21:04 +1100 Subject: [PATCH 06/16] Remove Func for default constructor invoking Fixing startup performance concerns --- src/mscorlib/src/System/Lazy.cs | 111 ++++++++++++++++++-------------- 1 file changed, 63 insertions(+), 48 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index fe32bc606057..9704363ccee4 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -45,6 +45,18 @@ namespace System public class Lazy : ISerializable { + private static T Construct() + { + try + { + return Activator.CreateInstance(); + } + catch (MissingMethodException) + { + throw new MissingMemberException(Environment.GetResourceString("Lazy_CreateValue_NoParameterlessCtorForT")); + } + } + /// /// ILazyItem<T> is used to determine the initialization logic that the Lazy object uses. /// @@ -75,7 +87,7 @@ internal interface ILazyItem /// An instance created with this constructor may be used concurrently from multiple threads. /// public Lazy() - : this(CreateInstance.Factory, LazyThreadSafetyMode.ExecutionAndPublication) + : this(null, LazyThreadSafetyMode.ExecutionAndPublication, true) { } @@ -108,7 +120,7 @@ public Lazy(T value) /// An instance created with this constructor may be used concurrently from multiple threads. /// public Lazy(Func valueFactory) - : this(valueFactory, LazyThreadSafetyMode.ExecutionAndPublication) + : this(valueFactory, LazyThreadSafetyMode.ExecutionAndPublication, false) { } @@ -119,7 +131,7 @@ public Lazy(Func valueFactory) /// true if this instance should be usable by multiple threads concurrently; false if the instance will only be used by one thread at a time. /// public Lazy(bool isThreadSafe) : - this(CreateInstance.Factory, GetMode(isThreadSafe)) + this(null, GetMode(isThreadSafe), true) { } @@ -130,7 +142,7 @@ public Lazy(bool isThreadSafe) : /// The lazy thread-safety mode mode /// mode contains an invalid valuee public Lazy(LazyThreadSafetyMode mode) : - this(CreateInstance.Factory, mode) + this(null, mode, true) { } @@ -146,7 +158,7 @@ public Lazy(LazyThreadSafetyMode mode) : /// is /// a null reference (Nothing in Visual Basic). public Lazy(Func valueFactory, bool isThreadSafe) : - this(valueFactory, GetMode(isThreadSafe)) + this(valueFactory, GetMode(isThreadSafe), false) { } @@ -162,8 +174,13 @@ public Lazy(Func valueFactory, bool isThreadSafe) : /// a null reference (Nothing in Visual Basic). /// mode contains an invalid value. public Lazy(Func valueFactory, LazyThreadSafetyMode mode) + : this(valueFactory, mode, false) { - if (valueFactory == null) + } + + private Lazy(Func valueFactory, LazyThreadSafetyMode mode, bool useDefaultConstructor) + { + if (!useDefaultConstructor && valueFactory == null) throw new ArgumentNullException(nameof(valueFactory)); m_factory = valueFactory; @@ -171,15 +188,15 @@ public Lazy(Func valueFactory, LazyThreadSafetyMode mode) switch (mode) { case LazyThreadSafetyMode.None: - m_implementation = None.Instance; + m_implementation = useDefaultConstructor ? None.ViaDefaultConstructor : None.ViaFactory; break; case LazyThreadSafetyMode.PublicationOnly: - m_implementation = PublicationOnly.Instance; + m_implementation = useDefaultConstructor ? PublicationOnly.ViaDefaultConstructor : PublicationOnly.ViaFactory; break; case LazyThreadSafetyMode.ExecutionAndPublication: - m_implementation = new ExecutionAndPublication(); + m_implementation = new ExecutionAndPublication(useDefaultConstructor); break; default: @@ -187,32 +204,11 @@ public Lazy(Func valueFactory, LazyThreadSafetyMode mode) } } -#region constructor helpers - - private static class CreateInstance - { - private static T Construct() - { - try - { - return (T)(Activator.CreateInstance(typeof(T))); - } - catch (MissingMethodException) - { - throw new MissingMemberException(Environment.GetResourceString("Lazy_CreateValue_NoParameterlessCtorForT")); - } - } - - internal readonly static Func Factory = Construct; - } - private static LazyThreadSafetyMode GetMode(bool isThreadSafe) { return isThreadSafe ? LazyThreadSafetyMode.ExecutionAndPublication : LazyThreadSafetyMode.None; } -#endregion - private sealed class LazyException : ILazyItem { private readonly System.Runtime.ExceptionServices.ExceptionDispatchInfo m_exceptionInfo; @@ -242,11 +238,11 @@ T ILazyItem.GetValue(Lazy owner) /// The object factory used to create the underlying object /// The mode of the Lazy object /// The underlying object - private T CreateValue(LazyThreadSafetyMode mode) + private T CreateValue(LazyThreadSafetyMode mode, bool useDefaultConstructor) { var factory = m_factory; - if (!ReferenceEquals(CreateInstance.Factory, factory)) + if (!useDefaultConstructor) { if (factory == null) throw new InvalidOperationException(Environment.GetResourceString("Lazy_Value_RecursiveCallsToValue")); @@ -255,11 +251,11 @@ private T CreateValue(LazyThreadSafetyMode mode) try { - m_value = factory(); + m_value = useDefaultConstructor ? Construct() : factory(); m_implementation = null; return m_value; } - catch (Exception exception) when (!ReferenceEquals(CreateInstance.Factory, factory)) + catch (Exception exception) when (!useDefaultConstructor) { m_implementation = new LazyException(exception, mode); throw; @@ -268,13 +264,19 @@ private T CreateValue(LazyThreadSafetyMode mode) private sealed class None : ILazyItem { - private None() { } + private bool UseDefaultConstructor { get; } - internal static ILazyItem Instance = new None(); + private None(bool useDefaultConstructor) + { + UseDefaultConstructor = useDefaultConstructor; + } + + internal static ILazyItem ViaDefaultConstructor = new None(true); + internal static ILazyItem ViaFactory = new None(false); T ILazyItem.GetValue(Lazy owner) { - return owner.CreateValue(LazyThreadSafetyMode.None); + return owner.CreateValue(LazyThreadSafetyMode.None, UseDefaultConstructor); } LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) @@ -286,21 +288,28 @@ LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) bool ILazyItem.GetIsValueFaulted(Lazy owner) { return false; } } - private T CreateValueExecutionAndPublication(ExecutionAndPublication sync) + private T CreateValueExecutionAndPublication(ExecutionAndPublication sync, bool useDefaultConstructor) { lock (sync) // we're safe to lock on "this" as object is an private object used by Lazy { // it's possible for multiple calls to have piled up behind the lock, so we need to check // to see if the ExecutionAndPublication object is still the current implementation. - return ReferenceEquals(m_implementation, sync) ? CreateValue(LazyThreadSafetyMode.ExecutionAndPublication) : Value; + return ReferenceEquals(m_implementation, sync) ? CreateValue(LazyThreadSafetyMode.ExecutionAndPublication, useDefaultConstructor) : Value; } } private sealed class ExecutionAndPublication : ILazyItem { + private bool UseDefaultConstructor { get; } + + internal ExecutionAndPublication(bool useDefaultConstructor) + { + UseDefaultConstructor = useDefaultConstructor; + } + T ILazyItem.GetValue(Lazy owner) { - return owner.CreateValueExecutionAndPublication(this); + return owner.CreateValueExecutionAndPublication(this, UseDefaultConstructor); } LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) @@ -319,21 +328,21 @@ LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) /// The object factory used to create the underlying object /// The publication object, used for synchronisation and comparison /// The underlying object - private T CreateValuePublicationOnly() + private T CreateValuePublicationOnly(ILazyItem initializer, bool useDefaultConstructor) { var factory = m_factory; - if (factory == null) + if (!useDefaultConstructor && factory == null) return PublicationOnlyWaiter.Instance.GetValue(this); - var possibleValue = factory(); + var possibleValue = useDefaultConstructor ? Construct() : factory(); try {} finally { // we run this in a finally block to ensure that we don't get a partial completion due // to a Thread.Abort, which could mean that other threads might be left in infinite loops - var previous = Interlocked.CompareExchange(ref m_implementation, PublicationOnlyWaiter.Instance, PublicationOnly.Instance); - if (previous == PublicationOnly.Instance) + var previous = Interlocked.CompareExchange(ref m_implementation, PublicationOnlyWaiter.Instance, initializer); + if (previous == initializer) { m_value = possibleValue; m_factory = null; @@ -346,13 +355,19 @@ private T CreateValuePublicationOnly() private sealed class PublicationOnly : ILazyItem { - private PublicationOnly() { } + private bool UseDefaultConstructor { get; } + + private PublicationOnly(bool useDefaultConstructor) + { + UseDefaultConstructor = useDefaultConstructor; + } - internal static ILazyItem Instance = new PublicationOnly(); + internal static ILazyItem ViaDefaultConstructor = new PublicationOnly(true); + internal static ILazyItem ViaFactory = new PublicationOnly(false); T ILazyItem.GetValue(Lazy owner) { - return owner.CreateValuePublicationOnly(); + return owner.CreateValuePublicationOnly(this, UseDefaultConstructor); } LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) From 21040379afc003ad5592f06c8ebcfa1d4772a569 Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Mon, 23 Jan 2017 19:26:54 +1100 Subject: [PATCH 07/16] Minimising the JIT startup time --- src/mscorlib/src/System/Lazy.cs | 347 +++++++++++++------------------- 1 file changed, 145 insertions(+), 202 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index 9704363ccee4..6d33023fff6f 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -27,6 +27,83 @@ namespace System { + class LazyImplementation + { + public readonly static LazyImplementation NoneViaConstructor = new LazyImplementation(LazyThreadSafetyMode.None, true); + public readonly static LazyImplementation NoneViaFactory = new LazyImplementation(LazyThreadSafetyMode.None, false); + public readonly static LazyImplementation PublicationOnlyViaConstructor = new LazyImplementation(LazyThreadSafetyMode.PublicationOnly, true); + public readonly static LazyImplementation PublicationOnlyViaFactory = new LazyImplementation(LazyThreadSafetyMode.PublicationOnly, false); + public readonly static LazyImplementation PublicationOnlySpinWait = new LazyImplementation(); + + private readonly LazyThreadSafetyMode m_mode; + private readonly bool m_publicationOnlyWaiting; + private readonly bool m_useDefaultConstructor; + private readonly System.Runtime.ExceptionServices.ExceptionDispatchInfo m_exceptionDispatch; + + /// + /// Constructor used for lazy construction + /// + /// + /// + public LazyImplementation(LazyThreadSafetyMode mode, bool useDefaultConstructor) + { + m_mode = mode; + m_publicationOnlyWaiting = false; + m_useDefaultConstructor = useDefaultConstructor; + m_exceptionDispatch = null; + } + + /// + /// Constructor used for exceptions + /// + /// + /// + public LazyImplementation(LazyThreadSafetyMode mode, Exception exception) + { + m_mode = mode; + m_publicationOnlyWaiting = false; + m_useDefaultConstructor = false; + m_exceptionDispatch = System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(exception); + } + + /// + /// Constructor used for the spin state that is required for PublicationOnly to funciton correctly + /// + private LazyImplementation() + { + m_mode = LazyThreadSafetyMode.PublicationOnly; + m_publicationOnlyWaiting = true; + m_useDefaultConstructor = false; + m_exceptionDispatch = null; + } + + public T GetValue(Lazy owner) + { + // If we are an exception-based then we throw that + if (m_exceptionDispatch != null) + m_exceptionDispatch.Throw(); + + if (m_mode == LazyThreadSafetyMode.ExecutionAndPublication) + return owner.PopulateValueExecutionAndPublication(this, m_useDefaultConstructor); + + if (m_mode == LazyThreadSafetyMode.None) + return owner.PopulateValue(LazyThreadSafetyMode.None, m_useDefaultConstructor); + + // Otherwise we're PublicationOnly, but we could be the creator or the spinner + + if (m_publicationOnlyWaiting) + { + owner.WaitForPublicationOnly(); + return owner.Value; + } + + return owner.PopulateValuePublicationOnly(this, PublicationOnlySpinWait, m_useDefaultConstructor); + } + + public LazyThreadSafetyMode GetMode() { return m_mode; } + public bool GetIsValueFaulted() { return m_exceptionDispatch != null; } + } + /// /// Provides support for lazy initialization. /// @@ -45,11 +122,11 @@ namespace System public class Lazy : ISerializable { - private static T Construct() + private static T CreateViaDefaultConstructor() { try { - return Activator.CreateInstance(); + return (T)Activator.CreateInstance(typeof(T)); } catch (MissingMethodException) { @@ -57,28 +134,16 @@ private static T Construct() } } - /// - /// ILazyItem<T> is used to determine the initialization logic that the Lazy object uses. - /// - /// - internal interface ILazyItem - { - T GetValue(Lazy owner); - bool GetIsValueCreated(Lazy owner); - bool GetIsValueFaulted(Lazy owner); - LazyThreadSafetyMode GetMode(Lazy owner); - } - // m_implementation, a volatile reference, is set to null after m_value has been set - private volatile ILazyItem m_implementation; + private volatile LazyImplementation m_implementation; - // we ensure that m_factory is allow garbage collector to clean up any + // we ensure that m_factory when finished is set to null to allow garbage collector to clean up + // any referenced items private Func m_factory; // m_value eventually stores the lazily created value. It is ready when m_implementation = null. private T m_value; - /// /// Initializes a new instance of the class that /// uses 's default constructor for lazy initialization. @@ -180,27 +245,26 @@ public Lazy(Func valueFactory, LazyThreadSafetyMode mode) private Lazy(Func valueFactory, LazyThreadSafetyMode mode, bool useDefaultConstructor) { - if (!useDefaultConstructor && valueFactory == null) + if (valueFactory == null && !useDefaultConstructor) throw new ArgumentNullException(nameof(valueFactory)); m_factory = valueFactory; - switch (mode) + if (mode == LazyThreadSafetyMode.None) { - case LazyThreadSafetyMode.None: - m_implementation = useDefaultConstructor ? None.ViaDefaultConstructor : None.ViaFactory; - break; - - case LazyThreadSafetyMode.PublicationOnly: - m_implementation = useDefaultConstructor ? PublicationOnly.ViaDefaultConstructor : PublicationOnly.ViaFactory; - break; - - case LazyThreadSafetyMode.ExecutionAndPublication: - m_implementation = new ExecutionAndPublication(useDefaultConstructor); - break; - - default: - throw new ArgumentOutOfRangeException(nameof(mode), Environment.GetResourceString("Lazy_ctor_ModeInvalid")); + m_implementation = useDefaultConstructor ? LazyImplementation.NoneViaConstructor : LazyImplementation.NoneViaFactory; + } + else if (mode == LazyThreadSafetyMode.PublicationOnly) + { + m_implementation = useDefaultConstructor ? LazyImplementation.PublicationOnlyViaConstructor : LazyImplementation.PublicationOnlyViaFactory; + } + else if (mode == LazyThreadSafetyMode.ExecutionAndPublication) + { + m_implementation = new LazyImplementation(LazyThreadSafetyMode.ExecutionAndPublication, useDefaultConstructor); + } + else + { + throw new Exception(); } } @@ -209,139 +273,81 @@ private static LazyThreadSafetyMode GetMode(bool isThreadSafe) return isThreadSafe ? LazyThreadSafetyMode.ExecutionAndPublication : LazyThreadSafetyMode.None; } - private sealed class LazyException : ILazyItem - { - private readonly System.Runtime.ExceptionServices.ExceptionDispatchInfo m_exceptionInfo; - private readonly LazyThreadSafetyMode m_mode; - - internal LazyException(Exception exception, LazyThreadSafetyMode mode) - { - m_exceptionInfo = System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(exception); - m_mode = mode; - } - - bool ILazyItem.GetIsValueCreated(Lazy owner) { return false; } - bool ILazyItem.GetIsValueFaulted(Lazy owner) { return true; } - LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) { return m_mode; } - - T ILazyItem.GetValue(Lazy owner) - { - m_exceptionInfo.Throw(); - return default(T); - } - } - /// /// Creates the underlying value using the factory object, placing the result inside this - /// object, and then used the Lazy objects ILazyItem<T> implemenation.to refer to it. + /// object, and then used the Lazy objects LazyWorker<T> implemenation.to refer to it. /// /// The object factory used to create the underlying object /// The mode of the Lazy object /// The underlying object - private T CreateValue(LazyThreadSafetyMode mode, bool useDefaultConstructor) + internal T PopulateValue(LazyThreadSafetyMode mode, bool useDefaultConstructor) { - var factory = m_factory; - - if (!useDefaultConstructor) - { - if (factory == null) - throw new InvalidOperationException(Environment.GetResourceString("Lazy_Value_RecursiveCallsToValue")); - m_factory = null; - } - - try + if (useDefaultConstructor) { - m_value = useDefaultConstructor ? Construct() : factory(); + m_value = CreateViaDefaultConstructor(); m_implementation = null; - return m_value; } - catch (Exception exception) when (!useDefaultConstructor) + else { - m_implementation = new LazyException(exception, mode); - throw; - } - } - - private sealed class None : ILazyItem - { - private bool UseDefaultConstructor { get; } - - private None(bool useDefaultConstructor) - { - UseDefaultConstructor = useDefaultConstructor; - } - - internal static ILazyItem ViaDefaultConstructor = new None(true); - internal static ILazyItem ViaFactory = new None(false); - - T ILazyItem.GetValue(Lazy owner) - { - return owner.CreateValue(LazyThreadSafetyMode.None, UseDefaultConstructor); - } + try + { + var factory = m_factory; + if (factory == null) + throw new InvalidOperationException(Environment.GetResourceString("Lazy_Value_RecursiveCallsToValue")); + m_factory = null; - LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) - { - return LazyThreadSafetyMode.None; + m_value = factory(); + m_implementation = null; + } + catch (Exception exception) + { + m_implementation = new LazyImplementation(mode, exception); + throw; + } } - - bool ILazyItem.GetIsValueCreated(Lazy owner) { return false; } - bool ILazyItem.GetIsValueFaulted(Lazy owner) { return false; } + return m_value; } - private T CreateValueExecutionAndPublication(ExecutionAndPublication sync, bool useDefaultConstructor) + internal T PopulateValueExecutionAndPublication(LazyImplementation sync, bool useDefaultConstructor) { lock (sync) // we're safe to lock on "this" as object is an private object used by Lazy { // it's possible for multiple calls to have piled up behind the lock, so we need to check // to see if the ExecutionAndPublication object is still the current implementation. - return ReferenceEquals(m_implementation, sync) ? CreateValue(LazyThreadSafetyMode.ExecutionAndPublication, useDefaultConstructor) : Value; - } - } - - private sealed class ExecutionAndPublication : ILazyItem - { - private bool UseDefaultConstructor { get; } - - internal ExecutionAndPublication(bool useDefaultConstructor) - { - UseDefaultConstructor = useDefaultConstructor; - } - - T ILazyItem.GetValue(Lazy owner) - { - return owner.CreateValueExecutionAndPublication(this, UseDefaultConstructor); - } - - LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) - { - return LazyThreadSafetyMode.ExecutionAndPublication; + if (ReferenceEquals(m_implementation, sync)) + return PopulateValue(LazyThreadSafetyMode.ExecutionAndPublication, useDefaultConstructor); + return Value; } - - bool ILazyItem.GetIsValueCreated(Lazy owner) { return false; } - bool ILazyItem.GetIsValueFaulted(Lazy owner) { return false; } } /// /// Creates the underlying value using the factory object into a helper object and, for the - /// first object to complete its factory, uses that objects ILazyItem<T> implementation + /// first object to complete its factory, uses that objects LazyWorker<T> implementation /// /// The object factory used to create the underlying object /// The publication object, used for synchronisation and comparison /// The underlying object - private T CreateValuePublicationOnly(ILazyItem initializer, bool useDefaultConstructor) + internal T PopulateValuePublicationOnly(LazyImplementation initializer, LazyImplementation spinner, bool useDefaultConstructor) { - var factory = m_factory; - if (!useDefaultConstructor && factory == null) - return PublicationOnlyWaiter.Instance.GetValue(this); - - var possibleValue = useDefaultConstructor ? Construct() : factory(); + T possibleValue; + if (useDefaultConstructor) + { + possibleValue = CreateViaDefaultConstructor(); + } + else + { + var factory = m_factory; + if (factory == null) + return spinner.GetValue(this); + possibleValue = factory(); + } - try {} + try { } finally { // we run this in a finally block to ensure that we don't get a partial completion due // to a Thread.Abort, which could mean that other threads might be left in infinite loops - var previous = Interlocked.CompareExchange(ref m_implementation, PublicationOnlyWaiter.Instance, initializer); + var previous = Interlocked.CompareExchange(ref m_implementation, spinner, initializer); if (previous == initializer) { m_value = possibleValue; @@ -353,76 +359,18 @@ private T CreateValuePublicationOnly(ILazyItem initializer, bool useDefaultConst return Value; } - private sealed class PublicationOnly : ILazyItem - { - private bool UseDefaultConstructor { get; } - - private PublicationOnly(bool useDefaultConstructor) - { - UseDefaultConstructor = useDefaultConstructor; - } - - internal static ILazyItem ViaDefaultConstructor = new PublicationOnly(true); - internal static ILazyItem ViaFactory = new PublicationOnly(false); - - T ILazyItem.GetValue(Lazy owner) - { - return owner.CreateValuePublicationOnly(this, UseDefaultConstructor); - } - - LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) - { - return LazyThreadSafetyMode.PublicationOnly; - } - - bool ILazyItem.GetIsValueCreated(Lazy owner) { return false; } - bool ILazyItem.GetIsValueFaulted(Lazy owner) { return false; } - } - - private void WaitForPublicationOnly() + internal void WaitForPublicationOnly() { while (!ReferenceEquals(m_implementation, null)) { // CreateValuePublicationOnly temporarily sets m_implementation to "this". The Lazy implementation - // has an explicit iplementation of ILazyItem which just waits for the m_value to be set, which is + // has an explicit iplementation of LazyWorker which just waits for the m_value to be set, which is // signalled by m_implemenation then being set to null. Thread.Sleep(0); } } - class PublicationOnlyWaiter - : ILazyItem - { - private PublicationOnlyWaiter() { } - - internal static ILazyItem Instance = new PublicationOnlyWaiter(); - - T ILazyItem.GetValue(Lazy owner) - { - owner.WaitForPublicationOnly(); - return owner.Value; - } - - bool ILazyItem.GetIsValueCreated(Lazy owner) - { - owner.WaitForPublicationOnly(); - return owner.IsValueCreated; - } - - bool ILazyItem.GetIsValueFaulted(Lazy owner) - { - owner.WaitForPublicationOnly(); - return owner.IsValueFaulted; - } - - LazyThreadSafetyMode ILazyItem.GetMode(Lazy owner) - { - owner.WaitForPublicationOnly(); - return owner.Mode; - } - } - -#region Serialization + #region Serialization // to remain compatible with previous version, custom serialization has been added // which should be binary compatible. Only valid values were ever serialized. Exceptions // were thrown from the serializer, which halted it, if the Lazy object through an exception. @@ -454,7 +402,7 @@ void ISerializable.GetObjectData(SerializationInfo info, StreamingContext contex info.AddValue("m_boxed", boxed); } -#endregion + #endregion /// Forces initialization during serialization. /// The StreamingContext for the serialization operation. @@ -499,7 +447,7 @@ internal LazyThreadSafetyMode Mode var implementation = m_implementation; if (implementation == null) return LazyThreadSafetyMode.None; // we don't know the mode anymore - return implementation.GetMode(this); + return implementation.GetMode(); } } @@ -513,7 +461,7 @@ internal bool IsValueFaulted var implementation = m_implementation; if (implementation == null) return false; - return implementation.GetIsValueFaulted(this); + return implementation.GetIsValueFaulted(); } } @@ -528,13 +476,7 @@ internal bool IsValueFaulted /// public bool IsValueCreated { - get - { - var implementation = m_implementation; - if (implementation == null) - return true; - return implementation.GetIsValueCreated(this); - } + get { return m_implementation == null; } } /// Gets the lazily initialized value of the current Date: Tue, 24 Jan 2017 19:22:58 +1100 Subject: [PATCH 08/16] Moved non-generic functionality out of Lazy ...and into a helper class --- src/mscorlib/src/System/Lazy.cs | 400 ++++++++++++++++---------------- 1 file changed, 205 insertions(+), 195 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index 6d33023fff6f..c8fc45903aa7 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -27,30 +27,47 @@ namespace System { - class LazyImplementation + internal enum LazyState { - public readonly static LazyImplementation NoneViaConstructor = new LazyImplementation(LazyThreadSafetyMode.None, true); - public readonly static LazyImplementation NoneViaFactory = new LazyImplementation(LazyThreadSafetyMode.None, false); - public readonly static LazyImplementation PublicationOnlyViaConstructor = new LazyImplementation(LazyThreadSafetyMode.PublicationOnly, true); - public readonly static LazyImplementation PublicationOnlyViaFactory = new LazyImplementation(LazyThreadSafetyMode.PublicationOnly, false); - public readonly static LazyImplementation PublicationOnlySpinWait = new LazyImplementation(); + NoneViaConstructor = 0, + NoneViaFactory = 1, + NoneException = 2, + + PublicationOnlyViaConstructor = 3, + PublicationOnlyViaFactory = 4, + PublicationOnlyWait = 5, + PublicationOnlyException = 6, + + ExecutionAndPublicationViaConstructor = 7, + ExecutionAndPublicationViaFactory = 8, + ExecutionAndPublicationException = 9, + } + + internal class LazyNonGeneric + { + internal readonly static LazyNonGeneric NoneViaConstructor = new LazyNonGeneric(LazyState.NoneViaConstructor); + internal readonly static LazyNonGeneric NoneViaFactory = new LazyNonGeneric(LazyState.NoneViaFactory); + internal readonly static LazyNonGeneric PublicationOnlyViaConstructor = new LazyNonGeneric(LazyState.PublicationOnlyViaConstructor); + internal readonly static LazyNonGeneric PublicationOnlyViaFactory = new LazyNonGeneric(LazyState.PublicationOnlyViaFactory); + internal readonly static LazyNonGeneric PublicationOnlySpinWait = new LazyNonGeneric(LazyState.PublicationOnlyWait); + + internal LazyState State { get; } - private readonly LazyThreadSafetyMode m_mode; - private readonly bool m_publicationOnlyWaiting; - private readonly bool m_useDefaultConstructor; - private readonly System.Runtime.ExceptionServices.ExceptionDispatchInfo m_exceptionDispatch; + private readonly System.Runtime.ExceptionServices.ExceptionDispatchInfo _exceptionDispatch; /// - /// Constructor used for lazy construction + /// Constructor that defines the state /// - /// - /// - public LazyImplementation(LazyThreadSafetyMode mode, bool useDefaultConstructor) + /// + internal LazyNonGeneric(LazyState state) + { + State = state; + } + + private Exception InvalidLogic() { - m_mode = mode; - m_publicationOnlyWaiting = false; - m_useDefaultConstructor = useDefaultConstructor; - m_exceptionDispatch = null; + // correctly implemented, we should never create this exception + return new Exception("Invalid logic; execution should not get here"); } /// @@ -58,50 +75,110 @@ public LazyImplementation(LazyThreadSafetyMode mode, bool useDefaultConstructor) /// /// /// - public LazyImplementation(LazyThreadSafetyMode mode, Exception exception) + internal LazyNonGeneric(LazyThreadSafetyMode mode, Exception exception) { - m_mode = mode; - m_publicationOnlyWaiting = false; - m_useDefaultConstructor = false; - m_exceptionDispatch = System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(exception); + if (mode == LazyThreadSafetyMode.ExecutionAndPublication) + State = LazyState.ExecutionAndPublicationException; + else if (mode == LazyThreadSafetyMode.None) + State = LazyState.NoneException; + else if (mode == LazyThreadSafetyMode.PublicationOnly) + State = LazyState.PublicationOnlyException; + else + throw InvalidLogic(); + + _exceptionDispatch = System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(exception); } - /// - /// Constructor used for the spin state that is required for PublicationOnly to funciton correctly - /// - private LazyImplementation() + internal void ThrowException() + { + if (_exceptionDispatch == null) + throw InvalidLogic(); + + _exceptionDispatch.Throw(); + } + + private LazyThreadSafetyMode GetMode() { - m_mode = LazyThreadSafetyMode.PublicationOnly; - m_publicationOnlyWaiting = true; - m_useDefaultConstructor = false; - m_exceptionDispatch = null; + switch (State) + { + case LazyState.NoneViaConstructor: + case LazyState.NoneViaFactory: + case LazyState.NoneException: + return LazyThreadSafetyMode.None; + + case LazyState.PublicationOnlyViaConstructor: + case LazyState.PublicationOnlyViaFactory: + case LazyState.PublicationOnlyWait: + case LazyState.PublicationOnlyException: + return LazyThreadSafetyMode.PublicationOnly; + + case LazyState.ExecutionAndPublicationViaConstructor: + case LazyState.ExecutionAndPublicationViaFactory: + case LazyState.ExecutionAndPublicationException: + return LazyThreadSafetyMode.ExecutionAndPublication; + + default: + throw InvalidLogic(); + } } - public T GetValue(Lazy owner) + internal static LazyThreadSafetyMode GetMode(LazyNonGeneric state) { - // If we are an exception-based then we throw that - if (m_exceptionDispatch != null) - m_exceptionDispatch.Throw(); + if (state == null) + return LazyThreadSafetyMode.None; // we don't know the mode anymore + return state.GetMode(); + } - if (m_mode == LazyThreadSafetyMode.ExecutionAndPublication) - return owner.PopulateValueExecutionAndPublication(this, m_useDefaultConstructor); + private bool GetIsValueFaulted() + { + return _exceptionDispatch != null; + } - if (m_mode == LazyThreadSafetyMode.None) - return owner.PopulateValue(LazyThreadSafetyMode.None, m_useDefaultConstructor); + internal static bool GetIsValueFaulted(LazyNonGeneric state) + { + if (state == null) + return false; + return state.GetIsValueFaulted(); + } - // Otherwise we're PublicationOnly, but we could be the creator or the spinner + internal static LazyNonGeneric Create(LazyThreadSafetyMode mode, bool useDefaultConstructor) + { + if (mode == LazyThreadSafetyMode.None) + { + return useDefaultConstructor ? NoneViaConstructor : NoneViaFactory; + } - if (m_publicationOnlyWaiting) + if (mode == LazyThreadSafetyMode.PublicationOnly) { - owner.WaitForPublicationOnly(); - return owner.Value; + return useDefaultConstructor ? PublicationOnlyViaConstructor : PublicationOnlyViaFactory; } - return owner.PopulateValuePublicationOnly(this, PublicationOnlySpinWait, m_useDefaultConstructor); + if (mode == LazyThreadSafetyMode.ExecutionAndPublication) + { + // we need to create an object for ExecutionAndPublication because we use Monitor-based locking + var state = useDefaultConstructor ? LazyState.ExecutionAndPublicationViaConstructor : LazyState.ExecutionAndPublicationViaFactory; + return new LazyNonGeneric(state); + } + + throw new ArgumentOutOfRangeException(nameof(mode), Environment.GetResourceString("Lazy_ctor_ModeInvalid")); + } + + internal static object CreateViaDefaultConstructor(Type type) + { + try + { + return Activator.CreateInstance(type); + } + catch (MissingMethodException) + { + throw new MissingMemberException(Environment.GetResourceString("Lazy_CreateValue_NoParameterlessCtorForT")); + } } - public LazyThreadSafetyMode GetMode() { return m_mode; } - public bool GetIsValueFaulted() { return m_exceptionDispatch != null; } + internal static LazyThreadSafetyMode GetModeFromIsThreadSafe(bool isThreadSafe) + { + return isThreadSafe ? LazyThreadSafetyMode.ExecutionAndPublication : LazyThreadSafetyMode.None; + } } /// @@ -120,29 +197,23 @@ public T GetValue(Lazy owner) [DebuggerTypeProxy(typeof(System_LazyDebugView<>))] [DebuggerDisplay("ThreadSafetyMode={Mode}, IsValueCreated={IsValueCreated}, IsValueFaulted={IsValueFaulted}, Value={ValueForDebugDisplay}")] public class Lazy - : ISerializable { private static T CreateViaDefaultConstructor() { - try - { - return (T)Activator.CreateInstance(typeof(T)); - } - catch (MissingMethodException) - { - throw new MissingMemberException(Environment.GetResourceString("Lazy_CreateValue_NoParameterlessCtorForT")); - } + return (T)LazyNonGeneric.CreateViaDefaultConstructor(typeof(T)); } - // m_implementation, a volatile reference, is set to null after m_value has been set - private volatile LazyImplementation m_implementation; + // _state, a volatile reference, is set to null after m_value has been set + [NonSerialized] + private volatile LazyNonGeneric _state; // we ensure that m_factory when finished is set to null to allow garbage collector to clean up // any referenced items - private Func m_factory; + [NonSerialized] + private Func _factory; - // m_value eventually stores the lazily created value. It is ready when m_implementation = null. - private T m_value; + // m_value eventually stores the lazily created value. It is ready when _state = null. + private T _value; /// /// Initializes a new instance of the class that @@ -166,9 +237,7 @@ public Lazy() /// public Lazy(T value) { - m_value = value; - m_factory = null; - m_implementation = null; + _value = value; } /// @@ -196,7 +265,7 @@ public Lazy(Func valueFactory) /// true if this instance should be usable by multiple threads concurrently; false if the instance will only be used by one thread at a time. /// public Lazy(bool isThreadSafe) : - this(null, GetMode(isThreadSafe), true) + this(null, LazyNonGeneric.GetModeFromIsThreadSafe(isThreadSafe), true) { } @@ -223,7 +292,7 @@ public Lazy(LazyThreadSafetyMode mode) : /// is /// a null reference (Nothing in Visual Basic). public Lazy(Func valueFactory, bool isThreadSafe) : - this(valueFactory, GetMode(isThreadSafe), false) + this(valueFactory, LazyNonGeneric.GetModeFromIsThreadSafe(isThreadSafe), false) { } @@ -248,161 +317,115 @@ private Lazy(Func valueFactory, LazyThreadSafetyMode mode, bool useDefaultCon if (valueFactory == null && !useDefaultConstructor) throw new ArgumentNullException(nameof(valueFactory)); - m_factory = valueFactory; - - if (mode == LazyThreadSafetyMode.None) - { - m_implementation = useDefaultConstructor ? LazyImplementation.NoneViaConstructor : LazyImplementation.NoneViaFactory; - } - else if (mode == LazyThreadSafetyMode.PublicationOnly) - { - m_implementation = useDefaultConstructor ? LazyImplementation.PublicationOnlyViaConstructor : LazyImplementation.PublicationOnlyViaFactory; - } - else if (mode == LazyThreadSafetyMode.ExecutionAndPublication) - { - m_implementation = new LazyImplementation(LazyThreadSafetyMode.ExecutionAndPublication, useDefaultConstructor); - } - else - { - throw new Exception(); - } + _factory = valueFactory; + _state = LazyNonGeneric.Create(mode, useDefaultConstructor); } - private static LazyThreadSafetyMode GetMode(bool isThreadSafe) + private T ViaConstructor() { - return isThreadSafe ? LazyThreadSafetyMode.ExecutionAndPublication : LazyThreadSafetyMode.None; + _value = CreateViaDefaultConstructor(); + _state = null; + + return _value; } - /// - /// Creates the underlying value using the factory object, placing the result inside this - /// object, and then used the Lazy objects LazyWorker<T> implemenation.to refer to it. - /// - /// The object factory used to create the underlying object - /// The mode of the Lazy object - /// The underlying object - internal T PopulateValue(LazyThreadSafetyMode mode, bool useDefaultConstructor) + private T ViaFactory(LazyThreadSafetyMode mode) { - if (useDefaultConstructor) + try { - m_value = CreateViaDefaultConstructor(); - m_implementation = null; + var factory = _factory; + if (factory == null) + throw new InvalidOperationException(Environment.GetResourceString("Lazy_Value_RecursiveCallsToValue")); + _factory = null; + + _value = factory(); + _state = null; + + return _value; } - else + catch (Exception exception) { - try - { - var factory = m_factory; - if (factory == null) - throw new InvalidOperationException(Environment.GetResourceString("Lazy_Value_RecursiveCallsToValue")); - m_factory = null; - - m_value = factory(); - m_implementation = null; - } - catch (Exception exception) - { - m_implementation = new LazyImplementation(mode, exception); - throw; - } + _state = new LazyNonGeneric(mode, exception); + throw; } - return m_value; } - internal T PopulateValueExecutionAndPublication(LazyImplementation sync, bool useDefaultConstructor) + private T ExecutionAndPublication(LazyNonGeneric executionAndPublication, bool useDefaultConstructor) { - lock (sync) // we're safe to lock on "this" as object is an private object used by Lazy + lock (executionAndPublication) { // it's possible for multiple calls to have piled up behind the lock, so we need to check // to see if the ExecutionAndPublication object is still the current implementation. - if (ReferenceEquals(m_implementation, sync)) - return PopulateValue(LazyThreadSafetyMode.ExecutionAndPublication, useDefaultConstructor); + if (ReferenceEquals(_state, executionAndPublication)) + return useDefaultConstructor ? ViaConstructor() : ViaFactory(LazyThreadSafetyMode.ExecutionAndPublication); + return Value; } } - /// - /// Creates the underlying value using the factory object into a helper object and, for the - /// first object to complete its factory, uses that objects LazyWorker<T> implementation - /// - /// The object factory used to create the underlying object - /// The publication object, used for synchronisation and comparison - /// The underlying object - internal T PopulateValuePublicationOnly(LazyImplementation initializer, LazyImplementation spinner, bool useDefaultConstructor) + private T PublicationOnly(LazyNonGeneric publicationOnly, T possibleValue) { - T possibleValue; - if (useDefaultConstructor) - { - possibleValue = CreateViaDefaultConstructor(); - } - else - { - var factory = m_factory; - if (factory == null) - return spinner.GetValue(this); - possibleValue = factory(); - } - try { } finally { // we run this in a finally block to ensure that we don't get a partial completion due // to a Thread.Abort, which could mean that other threads might be left in infinite loops - var previous = Interlocked.CompareExchange(ref m_implementation, spinner, initializer); - if (previous == initializer) + var previous = Interlocked.CompareExchange(ref _state, LazyNonGeneric.PublicationOnlySpinWait, publicationOnly); + if (previous == publicationOnly) { - m_value = possibleValue; - m_factory = null; - m_implementation = null; + _value = possibleValue; + _factory = null; + _state = null; } } return Value; } - internal void WaitForPublicationOnly() + private T PublicationOnlyViaConstructor(LazyNonGeneric initializer) { - while (!ReferenceEquals(m_implementation, null)) - { - // CreateValuePublicationOnly temporarily sets m_implementation to "this". The Lazy implementation - // has an explicit iplementation of LazyWorker which just waits for the m_value to be set, which is - // signalled by m_implemenation then being set to null. - Thread.Sleep(0); - } + return PublicationOnly(initializer, CreateViaDefaultConstructor()); } - #region Serialization - // to remain compatible with previous version, custom serialization has been added - // which should be binary compatible. Only valid values were ever serialized. Exceptions - // were thrown from the serializer, which halted it, if the Lazy object through an exception. + private T PublicationOnlyViaFactory(LazyNonGeneric initializer) + { + var factory = _factory; + if (factory == null) + return PublicationOnlySpinWait(); + return PublicationOnly(initializer, factory()); + } - /// - /// wrapper class to box the initialized value, this is mainly created to avoid boxing/unboxing the value each time the value is called in case T is - /// a value type - /// - [Serializable] - class Boxed + private T PublicationOnlySpinWait() { - internal Boxed(T value) + var spinWait = new SpinWait(); + while (!ReferenceEquals(_state, null)) { - m_value = value; + // We get here when PublicationOnly temporarily sets _state to LazyNonGeneric.PublicationOnlySpinWait. + // This temporary state should be quickly followed by _state being set to null. + spinWait.SpinOnce(); } - internal T m_value; + return Value; } - public Lazy(SerializationInfo information, StreamingContext context) + private T LazyGetValue(LazyNonGeneric state) { - var boxed = (Boxed)information.GetValue("m_boxed", typeof(Boxed)); - m_value = boxed.m_value; - m_implementation = null; - } + switch (state.State) + { + case LazyState.NoneViaConstructor: return ViaConstructor(); + case LazyState.NoneViaFactory: return ViaFactory(LazyThreadSafetyMode.None); - void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context) - { - var boxed = new Boxed(Value); - info.AddValue("m_boxed", boxed); - } + case LazyState.PublicationOnlyViaConstructor: return PublicationOnlyViaConstructor(state); + case LazyState.PublicationOnlyViaFactory: return PublicationOnlyViaFactory(state); + case LazyState.PublicationOnlyWait: return PublicationOnlySpinWait(); - #endregion + case LazyState.ExecutionAndPublicationViaConstructor: return ExecutionAndPublication(state, true); + case LazyState.ExecutionAndPublicationViaFactory: return ExecutionAndPublication(state, false); + + default: + state.ThrowException(); + return default(T); + } + } /// Forces initialization during serialization. /// The StreamingContext for the serialization operation. @@ -442,13 +465,7 @@ internal T ValueForDebugDisplay /// internal LazyThreadSafetyMode Mode { - get - { - var implementation = m_implementation; - if (implementation == null) - return LazyThreadSafetyMode.None; // we don't know the mode anymore - return implementation.GetMode(); - } + get { return LazyNonGeneric.GetMode(_state); } } /// @@ -456,13 +473,7 @@ internal LazyThreadSafetyMode Mode /// internal bool IsValueFaulted { - get - { - var implementation = m_implementation; - if (implementation == null) - return false; - return implementation.GetIsValueFaulted(); - } + get { return LazyNonGeneric.GetIsValueFaulted(_state); } } /// Gets a value indicating whether the has been initialized. @@ -476,7 +487,7 @@ internal bool IsValueFaulted /// public bool IsValueCreated { - get { return m_implementation == null; } + get { return _state == null; } } /// Gets the lazily initialized value of the current Date: Wed, 22 Feb 2017 20:19:39 +1100 Subject: [PATCH 09/16] Code review changes from @jkotas and @stephentoub --- src/mscorlib/src/System/Lazy.cs | 249 +++++++++++++++++--------------- 1 file changed, 131 insertions(+), 118 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index c8fc45903aa7..60d3f5ede797 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -43,56 +43,59 @@ internal enum LazyState ExecutionAndPublicationException = 9, } - internal class LazyNonGeneric + internal class LazyHelper { - internal readonly static LazyNonGeneric NoneViaConstructor = new LazyNonGeneric(LazyState.NoneViaConstructor); - internal readonly static LazyNonGeneric NoneViaFactory = new LazyNonGeneric(LazyState.NoneViaFactory); - internal readonly static LazyNonGeneric PublicationOnlyViaConstructor = new LazyNonGeneric(LazyState.PublicationOnlyViaConstructor); - internal readonly static LazyNonGeneric PublicationOnlyViaFactory = new LazyNonGeneric(LazyState.PublicationOnlyViaFactory); - internal readonly static LazyNonGeneric PublicationOnlySpinWait = new LazyNonGeneric(LazyState.PublicationOnlyWait); + internal readonly static LazyHelper NoneViaConstructor = new LazyHelper(LazyState.NoneViaConstructor); + internal readonly static LazyHelper NoneViaFactory = new LazyHelper(LazyState.NoneViaFactory); + internal readonly static LazyHelper PublicationOnlyViaConstructor = new LazyHelper(LazyState.PublicationOnlyViaConstructor); + internal readonly static LazyHelper PublicationOnlyViaFactory = new LazyHelper(LazyState.PublicationOnlyViaFactory); + internal readonly static LazyHelper PublicationOnlyWaitForOtherThreadToPublish = new LazyHelper(LazyState.PublicationOnlyWait); internal LazyState State { get; } - private readonly System.Runtime.ExceptionServices.ExceptionDispatchInfo _exceptionDispatch; + private readonly ExceptionDispatchInfo _exceptionDispatch; /// /// Constructor that defines the state /// - /// - internal LazyNonGeneric(LazyState state) + internal LazyHelper(LazyState state) { State = state; } - private Exception InvalidLogic() - { - // correctly implemented, we should never create this exception - return new Exception("Invalid logic; execution should not get here"); - } - /// /// Constructor used for exceptions /// - /// - /// - internal LazyNonGeneric(LazyThreadSafetyMode mode, Exception exception) - { - if (mode == LazyThreadSafetyMode.ExecutionAndPublication) - State = LazyState.ExecutionAndPublicationException; - else if (mode == LazyThreadSafetyMode.None) - State = LazyState.NoneException; - else if (mode == LazyThreadSafetyMode.PublicationOnly) - State = LazyState.PublicationOnlyException; - else - throw InvalidLogic(); + internal LazyHelper(LazyThreadSafetyMode mode, Exception exception) + { + switch(mode) + { + case LazyThreadSafetyMode.ExecutionAndPublication: + State = LazyState.ExecutionAndPublicationException; + break; + + case LazyThreadSafetyMode.None: + State = LazyState.NoneException; + break; + + case LazyThreadSafetyMode.PublicationOnly: + State = LazyState.PublicationOnlyException; + break; - _exceptionDispatch = System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(exception); + default: + Debug.Assert(false, "internal constructor, this should never occur"); + break; + } + + _exceptionDispatch = ExceptionDispatchInfo.Capture(exception); } internal void ThrowException() { if (_exceptionDispatch == null) - throw InvalidLogic(); + { + Debug.Assert(false, "execution path is invalid"); + } _exceptionDispatch.Throw(); } @@ -118,14 +121,15 @@ private LazyThreadSafetyMode GetMode() return LazyThreadSafetyMode.ExecutionAndPublication; default: - throw InvalidLogic(); + Debug.Assert(false, "Invalid logic; State should always have a valid value"); + throw new ArgumentOutOfRangeException(); } } - internal static LazyThreadSafetyMode GetMode(LazyNonGeneric state) + internal static LazyThreadSafetyMode? GetMode(LazyHelper state) { if (state == null) - return LazyThreadSafetyMode.None; // we don't know the mode anymore + return null; // we don't know the mode anymore return state.GetMode(); } @@ -134,33 +138,31 @@ private bool GetIsValueFaulted() return _exceptionDispatch != null; } - internal static bool GetIsValueFaulted(LazyNonGeneric state) + internal static bool GetIsValueFaulted(LazyHelper state) { if (state == null) return false; return state.GetIsValueFaulted(); } - internal static LazyNonGeneric Create(LazyThreadSafetyMode mode, bool useDefaultConstructor) + internal static LazyHelper Create(LazyThreadSafetyMode mode, bool useDefaultConstructor) { - if (mode == LazyThreadSafetyMode.None) + switch (mode) { - return useDefaultConstructor ? NoneViaConstructor : NoneViaFactory; - } + case LazyThreadSafetyMode.None: + return useDefaultConstructor ? NoneViaConstructor : NoneViaFactory; - if (mode == LazyThreadSafetyMode.PublicationOnly) - { - return useDefaultConstructor ? PublicationOnlyViaConstructor : PublicationOnlyViaFactory; - } + case LazyThreadSafetyMode.PublicationOnly: + return useDefaultConstructor ? PublicationOnlyViaConstructor : PublicationOnlyViaFactory; - if (mode == LazyThreadSafetyMode.ExecutionAndPublication) - { - // we need to create an object for ExecutionAndPublication because we use Monitor-based locking - var state = useDefaultConstructor ? LazyState.ExecutionAndPublicationViaConstructor : LazyState.ExecutionAndPublicationViaFactory; - return new LazyNonGeneric(state); - } + case LazyThreadSafetyMode.ExecutionAndPublication: + // we need to create an object for ExecutionAndPublication because we use Monitor-based locking + var state = useDefaultConstructor ? LazyState.ExecutionAndPublicationViaConstructor : LazyState.ExecutionAndPublicationViaFactory; + return new LazyHelper(state); - throw new ArgumentOutOfRangeException(nameof(mode), Environment.GetResourceString("Lazy_ctor_ModeInvalid")); + default: + throw new ArgumentOutOfRangeException(nameof(mode), Environment.GetResourceString("Lazy_ctor_ModeInvalid")); + } } internal static object CreateViaDefaultConstructor(Type type) @@ -200,19 +202,19 @@ public class Lazy { private static T CreateViaDefaultConstructor() { - return (T)LazyNonGeneric.CreateViaDefaultConstructor(typeof(T)); + return (T)LazyHelper.CreateViaDefaultConstructor(typeof(T)); } // _state, a volatile reference, is set to null after m_value has been set [NonSerialized] - private volatile LazyNonGeneric _state; + private volatile LazyHelper _state; - // we ensure that m_factory when finished is set to null to allow garbage collector to clean up + // we ensure that _factory when finished is set to null to allow garbage collector to clean up // any referenced items [NonSerialized] private Func _factory; - // m_value eventually stores the lazily created value. It is ready when _state = null. + // m_value eventually stores the lazily created value. It is valid when _state = null. private T _value; /// @@ -254,7 +256,7 @@ public Lazy(T value) /// An instance created with this constructor may be used concurrently from multiple threads. /// public Lazy(Func valueFactory) - : this(valueFactory, LazyThreadSafetyMode.ExecutionAndPublication, false) + : this(valueFactory, LazyThreadSafetyMode.ExecutionAndPublication, useDefaultConstructor:false) { } @@ -265,7 +267,7 @@ public Lazy(Func valueFactory) /// true if this instance should be usable by multiple threads concurrently; false if the instance will only be used by one thread at a time. /// public Lazy(bool isThreadSafe) : - this(null, LazyNonGeneric.GetModeFromIsThreadSafe(isThreadSafe), true) + this(null, LazyHelper.GetModeFromIsThreadSafe(isThreadSafe), useDefaultConstructor:true) { } @@ -276,7 +278,7 @@ public Lazy(bool isThreadSafe) : /// The lazy thread-safety mode mode /// mode contains an invalid valuee public Lazy(LazyThreadSafetyMode mode) : - this(null, mode, true) + this(null, mode, useDefaultConstructor:true) { } @@ -292,7 +294,7 @@ public Lazy(LazyThreadSafetyMode mode) : /// is /// a null reference (Nothing in Visual Basic). public Lazy(Func valueFactory, bool isThreadSafe) : - this(valueFactory, LazyNonGeneric.GetModeFromIsThreadSafe(isThreadSafe), false) + this(valueFactory, LazyHelper.GetModeFromIsThreadSafe(isThreadSafe), useDefaultConstructor:false) { } @@ -308,7 +310,7 @@ public Lazy(Func valueFactory, bool isThreadSafe) : /// a null reference (Nothing in Visual Basic). /// mode contains an invalid value. public Lazy(Func valueFactory, LazyThreadSafetyMode mode) - : this(valueFactory, mode, false) + : this(valueFactory, mode, useDefaultConstructor:false) { } @@ -318,113 +320,133 @@ private Lazy(Func valueFactory, LazyThreadSafetyMode mode, bool useDefaultCon throw new ArgumentNullException(nameof(valueFactory)); _factory = valueFactory; - _state = LazyNonGeneric.Create(mode, useDefaultConstructor); + _state = LazyHelper.Create(mode, useDefaultConstructor); } - private T ViaConstructor() + private void ViaConstructor() { _value = CreateViaDefaultConstructor(); - _state = null; - - return _value; + _state = null; // volatile write, must occur after setting _value } - private T ViaFactory(LazyThreadSafetyMode mode) + private void ViaFactory(LazyThreadSafetyMode mode) { try { - var factory = _factory; + Func factory = _factory; if (factory == null) throw new InvalidOperationException(Environment.GetResourceString("Lazy_Value_RecursiveCallsToValue")); _factory = null; _value = factory(); - _state = null; - - return _value; + _state = null; // volatile write, must occur after setting _value } catch (Exception exception) { - _state = new LazyNonGeneric(mode, exception); + _state = new LazyHelper(mode, exception); throw; } } - private T ExecutionAndPublication(LazyNonGeneric executionAndPublication, bool useDefaultConstructor) + private void ExecutionAndPublication(LazyHelper executionAndPublication, bool useDefaultConstructor) { lock (executionAndPublication) { // it's possible for multiple calls to have piled up behind the lock, so we need to check // to see if the ExecutionAndPublication object is still the current implementation. if (ReferenceEquals(_state, executionAndPublication)) - return useDefaultConstructor ? ViaConstructor() : ViaFactory(LazyThreadSafetyMode.ExecutionAndPublication); - - return Value; + { + if (useDefaultConstructor) + { + ViaConstructor(); + } + else + { + ViaFactory(LazyThreadSafetyMode.ExecutionAndPublication); + } + } } } - private T PublicationOnly(LazyNonGeneric publicationOnly, T possibleValue) + private void PublicationOnly(LazyHelper publicationOnly, T possibleValue) { - try { } - finally + LazyHelper previous = Interlocked.CompareExchange(ref _state, LazyHelper.PublicationOnlyWaitForOtherThreadToPublish, publicationOnly); + if (previous == publicationOnly) { - // we run this in a finally block to ensure that we don't get a partial completion due - // to a Thread.Abort, which could mean that other threads might be left in infinite loops - var previous = Interlocked.CompareExchange(ref _state, LazyNonGeneric.PublicationOnlySpinWait, publicationOnly); - if (previous == publicationOnly) - { - _value = possibleValue; - _factory = null; - _state = null; - } + _factory = null; + _value = possibleValue; + _state = null; // volatile write, must occur after setting _value } - - return Value; } - private T PublicationOnlyViaConstructor(LazyNonGeneric initializer) + private void PublicationOnlyViaConstructor(LazyHelper initializer) { - return PublicationOnly(initializer, CreateViaDefaultConstructor()); + PublicationOnly(initializer, CreateViaDefaultConstructor()); } - private T PublicationOnlyViaFactory(LazyNonGeneric initializer) + private void PublicationOnlyViaFactory(LazyHelper initializer) { - var factory = _factory; + Func factory = _factory; if (factory == null) - return PublicationOnlySpinWait(); - return PublicationOnly(initializer, factory()); + { + PublicationOnlyWaitForOtherThreadToPublish(); + } + else + { + PublicationOnly(initializer, factory()); + } } - private T PublicationOnlySpinWait() + private void PublicationOnlyWaitForOtherThreadToPublish() { var spinWait = new SpinWait(); while (!ReferenceEquals(_state, null)) { - // We get here when PublicationOnly temporarily sets _state to LazyNonGeneric.PublicationOnlySpinWait. + // We get here when PublicationOnly temporarily sets _state to LazyHelper.PublicationOnlyWaitForOtherThreadToPublish. // This temporary state should be quickly followed by _state being set to null. spinWait.SpinOnce(); } - return Value; } - private T LazyGetValue(LazyNonGeneric state) + private T LazyGetValue() { + var state = _state; switch (state.State) { - case LazyState.NoneViaConstructor: return ViaConstructor(); - case LazyState.NoneViaFactory: return ViaFactory(LazyThreadSafetyMode.None); + case LazyState.NoneViaConstructor: + ViaConstructor(); + break; + + case LazyState.NoneViaFactory: + ViaFactory(LazyThreadSafetyMode.None); + break; + + case LazyState.PublicationOnlyViaConstructor: + PublicationOnlyViaConstructor(state); + break; + + case LazyState.PublicationOnlyViaFactory: + PublicationOnlyViaFactory(state); + break; - case LazyState.PublicationOnlyViaConstructor: return PublicationOnlyViaConstructor(state); - case LazyState.PublicationOnlyViaFactory: return PublicationOnlyViaFactory(state); - case LazyState.PublicationOnlyWait: return PublicationOnlySpinWait(); + case LazyState.PublicationOnlyWait: + PublicationOnlyWaitForOtherThreadToPublish(); + break; - case LazyState.ExecutionAndPublicationViaConstructor: return ExecutionAndPublication(state, true); - case LazyState.ExecutionAndPublicationViaFactory: return ExecutionAndPublication(state, false); + case LazyState.ExecutionAndPublicationViaConstructor: + ExecutionAndPublication(state, useDefaultConstructor:true); + break; + + case LazyState.ExecutionAndPublicationViaFactory: + ExecutionAndPublication(state, useDefaultConstructor:false); + break; default: state.ThrowException(); return default(T); } + + return Value; } /// Forces initialization during serialization. @@ -456,16 +478,16 @@ internal T ValueForDebugDisplay { return default(T); } - return Value; + return _value; } } /// /// Gets a value indicating whether this instance may be used concurrently from multiple threads. /// - internal LazyThreadSafetyMode Mode + internal LazyThreadSafetyMode? Mode { - get { return LazyNonGeneric.GetMode(_state); } + get { return LazyHelper.GetMode(_state); } } /// @@ -473,7 +495,7 @@ internal LazyThreadSafetyMode Mode /// internal bool IsValueFaulted { - get { return LazyNonGeneric.GetIsValueFaulted(_state); } + get { return LazyHelper.GetIsValueFaulted(_state); } } /// Gets a value indicating whether the has been initialized. @@ -512,16 +534,7 @@ public bool IsValueCreated /// from initialization delegate. /// [DebuggerBrowsable(DebuggerBrowsableState.Never)] - public T Value - { - get - { - var state = _state; - if (state == null) - return _value; - return LazyGetValue(state); - } - } + public T Value => _state == null ? _value : LazyGetValue(); } /// A debugger view of the Lazy<T> to surface additional debugging properties and @@ -552,7 +565,7 @@ public T Value } /// Returns the execution mode of the Lazy object - public LazyThreadSafetyMode Mode + public LazyThreadSafetyMode? Mode { get { return m_lazy.Mode; } } From 50b623c81ba50c92f34afa7123a907f52f96ff3f Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Wed, 22 Feb 2017 20:33:29 +1100 Subject: [PATCH 10/16] Missed naming an argument - fixed --- src/mscorlib/src/System/Lazy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index 60d3f5ede797..460f4584baaa 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -225,7 +225,7 @@ private static T CreateViaDefaultConstructor() /// An instance created with this constructor may be used concurrently from multiple threads. /// public Lazy() - : this(null, LazyThreadSafetyMode.ExecutionAndPublication, true) + : this(null, LazyThreadSafetyMode.ExecutionAndPublication, useDefaultConstructor:true) { } From 00531f2239594f42270159a57eb693eeb081b76b Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Wed, 1 Mar 2017 18:50:30 +1100 Subject: [PATCH 11/16] Tighter code around invalid code paths --- src/mscorlib/src/System/Lazy.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index 460f4584baaa..268923880c83 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -92,10 +92,7 @@ internal LazyHelper(LazyThreadSafetyMode mode, Exception exception) internal void ThrowException() { - if (_exceptionDispatch == null) - { - Debug.Assert(false, "execution path is invalid"); - } + Debug.Assert((_exceptionDispatch != null, "execution path is invalid"); _exceptionDispatch.Throw(); } @@ -122,7 +119,7 @@ private LazyThreadSafetyMode GetMode() default: Debug.Assert(false, "Invalid logic; State should always have a valid value"); - throw new ArgumentOutOfRangeException(); + return default(LazyThreadSafetyMode); } } @@ -443,7 +440,7 @@ private T LazyGetValue() default: state.ThrowException(); - return default(T); + break; } return Value; From b67b66935b788bf73c4968170f0a749c499ac7e0 Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Wed, 1 Mar 2017 19:16:26 +1100 Subject: [PATCH 12/16] Fixed race condition Race condition was introduced when state wasn't passed to LazyGetValue. Also just made LazyGetValue into void CreateValue, as it was internally recursively calling back to Value, so I think this is easier to grok threading implications (i.e. why it wasn't just returning _value.) --- src/mscorlib/src/System/Lazy.cs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index 268923880c83..d17ac3df937a 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -405,9 +405,15 @@ private void PublicationOnlyWaitForOtherThreadToPublish() } } - private T LazyGetValue() + private void CreateValue() { + // we have to create a copy of state here, and use the copy exclusively from here on in + // so as to ensure thread safety. var state = _state; + + if (state == null) + return; + switch (state.State) { case LazyState.NoneViaConstructor: @@ -442,8 +448,6 @@ private T LazyGetValue() state.ThrowException(); break; } - - return Value; } /// Forces initialization during serialization. @@ -531,7 +535,17 @@ public bool IsValueCreated /// from initialization delegate. /// [DebuggerBrowsable(DebuggerBrowsableState.Never)] - public T Value => _state == null ? _value : LazyGetValue(); + public T Value + { + while (true) + { + if (_state == null) + return _value; + + CreateValue(); + Debug.Assert(_state == null); + } + } } /// A debugger view of the Lazy<T> to surface additional debugging properties and From dbfa2f8b4c075412026b2f6607effffb4f41287a Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Wed, 1 Mar 2017 19:24:45 +1100 Subject: [PATCH 13/16] Added descriptive comment for LazyHelper --- src/mscorlib/src/System/Lazy.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index d17ac3df937a..15ec2ee01397 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -43,6 +43,14 @@ internal enum LazyState ExecutionAndPublicationException = 9, } + /// + /// LazyHelper serves multiples purposes + /// - minimizing code size of Lazy<T> by implementing as much of the code that is not generic + /// this reduces generic code bloat, making faster class initialization + /// - contains singleton objects that are used to handle threading primitives for PublicationOnly mode + /// - allows for instantiation for ExecutionAndPublication so as to create an object for locking on + /// - holds exception information. + /// internal class LazyHelper { internal readonly static LazyHelper NoneViaConstructor = new LazyHelper(LazyState.NoneViaConstructor); From 00e949fee63fb76d9698d4a53ea4140ace92ea0e Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Wed, 1 Mar 2017 19:29:02 +1100 Subject: [PATCH 14/16] Expression-bodied functions --- src/mscorlib/src/System/Lazy.cs | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index 15ec2ee01397..67ec1d72195c 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -138,17 +138,7 @@ private LazyThreadSafetyMode GetMode() return state.GetMode(); } - private bool GetIsValueFaulted() - { - return _exceptionDispatch != null; - } - - internal static bool GetIsValueFaulted(LazyHelper state) - { - if (state == null) - return false; - return state.GetIsValueFaulted(); - } + internal static bool GetIsValueFaulted() => state?._exceptionDispatch != null; internal static LazyHelper Create(LazyThreadSafetyMode mode, bool useDefaultConstructor) { @@ -494,18 +484,12 @@ internal T ValueForDebugDisplay /// /// Gets a value indicating whether this instance may be used concurrently from multiple threads. /// - internal LazyThreadSafetyMode? Mode - { - get { return LazyHelper.GetMode(_state); } - } + internal LazyThreadSafetyMode? Mode => LazyHelper.GetMode(_state); /// /// Gets whether the value creation is faulted or not /// - internal bool IsValueFaulted - { - get { return LazyHelper.GetIsValueFaulted(_state); } - } + internal bool IsValueFaulted => LazyHelper.GetIsValueFaulted(_state); /// Gets a value indicating whether the has been initialized. /// @@ -516,10 +500,7 @@ internal bool IsValueFaulted /// a value being produced or an exception being thrown. If an exception goes unhandled during initialization, /// will return false. /// - public bool IsValueCreated - { - get { return _state == null; } - } + public bool IsValueCreated => _state == null; /// Gets the lazily initialized value of the current . From 77d3a7f746ab533fa56991d791a4c05083924d1c Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Wed, 1 Mar 2017 19:35:29 +1100 Subject: [PATCH 15/16] Fix compilation errors due to negligence... --- src/mscorlib/src/System/Lazy.cs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index 67ec1d72195c..b17cbdaaf4ad 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -100,7 +100,7 @@ internal LazyHelper(LazyThreadSafetyMode mode, Exception exception) internal void ThrowException() { - Debug.Assert((_exceptionDispatch != null, "execution path is invalid"); + Debug.Assert(_exceptionDispatch != null, "execution path is invalid"); _exceptionDispatch.Throw(); } @@ -138,7 +138,7 @@ private LazyThreadSafetyMode GetMode() return state.GetMode(); } - internal static bool GetIsValueFaulted() => state?._exceptionDispatch != null; + internal static bool GetIsValueFaulted(LazyHelper state) => state?._exceptionDispatch != null; internal static LazyHelper Create(LazyThreadSafetyMode mode, bool useDefaultConstructor) { @@ -525,14 +525,17 @@ internal T ValueForDebugDisplay /// [DebuggerBrowsable(DebuggerBrowsableState.Never)] public T Value - { - while (true) + { + get { - if (_state == null) - return _value; + while (true) + { + if (_state == null) + return _value; - CreateValue(); - Debug.Assert(_state == null); + CreateValue(); + Debug.Assert(_state == null); + } } } } From de2a4a77cd4ebdf601c578e433d721416281c9ce Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Sat, 4 Mar 2017 10:08:41 +1100 Subject: [PATCH 16/16] Removed looping from Value to enable(?) inlining --- src/mscorlib/src/System/Lazy.cs | 76 ++++++++++++++------------------- 1 file changed, 31 insertions(+), 45 deletions(-) diff --git a/src/mscorlib/src/System/Lazy.cs b/src/mscorlib/src/System/Lazy.cs index b17cbdaaf4ad..58fdd4a38a44 100644 --- a/src/mscorlib/src/System/Lazy.cs +++ b/src/mscorlib/src/System/Lazy.cs @@ -18,7 +18,6 @@ using System.Runtime; using System.Runtime.InteropServices; using System.Security; -using System.Security.Permissions; using System.Diagnostics; using System.Runtime.Serialization; using System.Threading; @@ -403,49 +402,49 @@ private void PublicationOnlyWaitForOtherThreadToPublish() } } - private void CreateValue() + private T CreateValue() { // we have to create a copy of state here, and use the copy exclusively from here on in // so as to ensure thread safety. var state = _state; - - if (state == null) - return; - - switch (state.State) + if (state != null) { - case LazyState.NoneViaConstructor: - ViaConstructor(); - break; + switch (state.State) + { + case LazyState.NoneViaConstructor: + ViaConstructor(); + break; - case LazyState.NoneViaFactory: - ViaFactory(LazyThreadSafetyMode.None); - break; + case LazyState.NoneViaFactory: + ViaFactory(LazyThreadSafetyMode.None); + break; - case LazyState.PublicationOnlyViaConstructor: - PublicationOnlyViaConstructor(state); - break; + case LazyState.PublicationOnlyViaConstructor: + PublicationOnlyViaConstructor(state); + break; - case LazyState.PublicationOnlyViaFactory: - PublicationOnlyViaFactory(state); - break; + case LazyState.PublicationOnlyViaFactory: + PublicationOnlyViaFactory(state); + break; - case LazyState.PublicationOnlyWait: - PublicationOnlyWaitForOtherThreadToPublish(); - break; + case LazyState.PublicationOnlyWait: + PublicationOnlyWaitForOtherThreadToPublish(); + break; - case LazyState.ExecutionAndPublicationViaConstructor: - ExecutionAndPublication(state, useDefaultConstructor:true); - break; + case LazyState.ExecutionAndPublicationViaConstructor: + ExecutionAndPublication(state, useDefaultConstructor:true); + break; - case LazyState.ExecutionAndPublicationViaFactory: - ExecutionAndPublication(state, useDefaultConstructor:false); - break; + case LazyState.ExecutionAndPublicationViaFactory: + ExecutionAndPublication(state, useDefaultConstructor:false); + break; - default: - state.ThrowException(); - break; + default: + state.ThrowException(); + break; + } } + return Value; } /// Forces initialization during serialization. @@ -524,20 +523,7 @@ internal T ValueForDebugDisplay /// from initialization delegate. /// [DebuggerBrowsable(DebuggerBrowsableState.Never)] - public T Value - { - get - { - while (true) - { - if (_state == null) - return _value; - - CreateValue(); - Debug.Assert(_state == null); - } - } - } + public T Value => _state == null ? _value : CreateValue(); } /// A debugger view of the Lazy<T> to surface additional debugging properties and