Improve/Fix ViewableProperty

rd-net/Lifetimes/Collections/Viewable/ViewableProperty.cs

@@ -1,8 +1,10 @@
 using System;
 using System.Collections.Generic;
+using System.Threading;
 using JetBrains.Core;
 using JetBrains.Diagnostics;
 using JetBrains.Lifetimes;
+using JetBrains.Util.Internal;
 
 namespace JetBrains.Collections.Viewable
 {
@@ -12,11 +14,35 @@ namespace JetBrains.Collections.Viewable
   /// <typeparam name="T"></typeparam>
   public class ViewableProperty<T> : IViewableProperty<T>
   {
+    private static readonly bool ourIsReadWriteAtomic = Memory.IsReadWriteAtomic<T>();
+
     private readonly Signal<T> myChange = new Signal<T>();
 
+    private T myValue = default!;
+    private volatile bool myHasValue;
+
     public ISource<T> Change => myChange;
 
-    public Maybe<T> Maybe { get; private set; }
+    public Maybe<T> Maybe
+    {
+      get
+      {
+        if (myHasValue)
+        {
+          if (ourIsReadWriteAtomic)
+          {
+            return new Maybe<T>(myValue);
+          }
+
+          lock (myChange)
+          {
+            return new Maybe<T>(myValue);
+          }
+        }
+
+        return Maybe<T>.None;
+      }
+    }
 
     public ViewableProperty() {}
 
@@ -36,7 +62,13 @@ public virtual T Value
         lock (myChange)
         {
           if (Maybe.HasValue && EqualityComparer<T>.Default.Equals(Maybe.Value, value)) return;
-          Maybe = new Maybe<T>(value);
+          myValue = value;
+          myHasValue = true;
+
+          // After optimizing signal, `Fire` no longer provides a full memory fence (triggered by `Interlocked.CompareExchange`).
+          // This caused our tests to become flaky because we rely on `Fire(value)` being observed strictly after `myValue = value`;
+          // To enforce this ordering, we explicitly add a memory barrier here.
+          Interlocked.MemoryBarrier();
           myChange.Fire(value);
         }
       }

rd-net/Lifetimes/Lifetimes.csproj

@@ -46,6 +46,7 @@
 
     <ItemGroup>
         <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0-beta2-19367-01" PrivateAssets="all" />
+        <PackageReference Include="System.Reflection.Emit.Lightweight" Version="4.6.0" Condition="'$(TargetFramework)' == 'netstandard2.0'" />
     </ItemGroup>
 
     <ItemGroup>

rd-net/Lifetimes/Util/Memory.cs

@@ -1,3 +1,5 @@
+using System;
+using System.Reflection.Emit;
 using System.Runtime.CompilerServices;
 using System.Threading;
 
@@ -9,7 +11,7 @@ public class Memory
     public static unsafe void CopyMemory(byte* src, byte* dest, int len)
     {
 
-      
+
       if(len >= 0x10)
       {
         do
@@ -63,7 +65,7 @@ public static T VolatileRead<T>(ref T location) where T : class
     {
       return Volatile.Read(ref location);
     }
-    
+
     [MethodImpl(MethodImplAdvancedOptions.AggressiveInlining)]
     public static void VolatileWrite<T>(ref T location, T value) where T : class
     {
@@ -76,23 +78,73 @@ public static int VolatileRead(ref int location)
     {
       return Volatile.Read(ref location);
     }
-    
+
     [MethodImpl(MethodImplAdvancedOptions.AggressiveInlining)]
     public static void VolatileWrite(ref int location, int value)
     {
       Volatile.Write(ref location, value);
     }
-    
+
     [MethodImpl(MethodImplAdvancedOptions.AggressiveInlining)]
     public static bool VolatileRead(ref bool location)
     {
       return Volatile.Read(ref location);
     }
-    
+
     [MethodImpl(MethodImplAdvancedOptions.AggressiveInlining)]
     public static void VolatileWrite(ref bool location, bool value)
     {
       Volatile.Write(ref location, value);
     }
+
+    /// <summary>
+    /// Returns the managed slot size of <typeparamref name="T"/>: for value types, this is the struct size
+    /// including trailing padding; for reference types, this is <see cref="IntPtr.Size"/> (the reference
+    /// slot size, not the heap object size).
+    /// </summary>
+    public static int SizeOf<T>() => SizeOfCache<T>.Size;
+
+    public static bool IsReadWriteAtomic<T>()
+    {
+      return IsReadWriteAtomicCache<T>.IsReadWriteAtomic;
+    }
+
+    private static readonly int MaxAtomicSize = IntPtr.Size;
+
+    private static class SizeOfCache<T>
+    {
+      public static readonly int Size = Compute();
+
+      private static int Compute()
+      {
+#if NET5_0_OR_GREATER
+        return Unsafe.SizeOf<T>();
+#else
+        var dm = new DynamicMethod("SizeOf", typeof(int), Type.EmptyTypes, typeof(Memory).Module, true);
+        var il = dm.GetILGenerator();
+        il.Emit(OpCodes.Sizeof, typeof(T));
+        il.Emit(OpCodes.Ret);
+        return ((Func<int>)dm.CreateDelegate(typeof(Func<int>)))();
+#endif
+      }
+    }
+
+    private static class IsReadWriteAtomicCache<T>
+    {
+      public static readonly bool IsReadWriteAtomic = Compute();
+
+      private static bool Compute()
+      {
+        var type = typeof(T);
+        if (!type.IsValueType) return true;
+
+        var layoutAttr = type.StructLayoutAttribute;
+        if (layoutAttr != null && layoutAttr.Pack != 0 && layoutAttr.Pack < MaxAtomicSize)
+          return false;
+
+        var size = SizeOf<T>();
+        return size <= MaxAtomicSize;
+      }
+    }
   }
-}
+}