Improve/Fix ViewableProperty V2

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,36 @@ 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() {}
 
@@ -35,8 +62,13 @@ public virtual T Value
       {
         lock (myChange)
         {
-          if (Maybe.HasValue && EqualityComparer<T>.Default.Equals(Maybe.Value, value)) return;
-          Maybe = new Maybe<T>(value);
+          if (myHasValue && EqualityComparer<T>.Default.Equals(myValue, value)) return;
+          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/Util/Memory.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Runtime.CompilerServices;
 using System.Threading;
 
@@ -94,5 +95,53 @@ public static void VolatileWrite(ref bool location, bool value)
     {
       Volatile.Write(ref location, value);
     }
+
+    public static bool IsReadWriteAtomic<T>()
+    {
+      return IsReadWriteAtomicCache<T>.IsReadWriteAtomic;
+    }
+
+    private static class IsReadWriteAtomicCache<T>
+    {
+      // ReSharper disable once StaticMemberInGenericType
+      public static readonly bool IsReadWriteAtomic = Compute();
+
+      private static bool Compute()
+      {
+        //According to runtime specification
+        //https://github.com/dotnet/runtime/blob/main/docs/design/specs/Memory-model.md#atomic-memory-accesses
+        //Managed references accesses are atomic
+        if (!typeof(T).IsValueType)
+          return true;
+        //Otherwise, only primitive and Enum types with size up to the platform pointer size accesses are atomic
+        if (!typeof(T).IsPrimitive && !typeof(T).IsEnum)
+          return false;
+
+        //Native integer primitive types
+        if (typeof(T) == typeof(IntPtr) || typeof(T) == typeof(UIntPtr))
+          return true;
+
+        //Other primitive types and Enum types (via underlying primitive type)
+        switch (Type.GetTypeCode(typeof(T)))
+        {
+          case TypeCode.Boolean:
+          case TypeCode.SByte:
+          case TypeCode.Byte:
+          case TypeCode.Char:
+          case TypeCode.Int16:
+          case TypeCode.UInt16:
+          case TypeCode.Int32:
+          case TypeCode.UInt32:
+          case TypeCode.Single:
+            return true;
+          case TypeCode.Int64:
+          case TypeCode.UInt64:
+          case TypeCode.Double:
+            return IntPtr.Size == 8;
+          default:
+            return false;
+        }
+      }
+    }
   }
 }

rd-net/Test.Lifetimes/Utils/MemoryTest.cs

@@ -0,0 +1,83 @@
+using System;
+using JetBrains.Util.Internal;
+using NUnit.Framework;
+
+namespace Test.Lifetimes.Utils
+{
+  public class MemoryTest
+  {
+    #region TestTypes
+    private enum ByteEnum : byte { Value = 1 }
+    private enum IntEnum : int { Value = 1 }
+    private enum UIntEnum : int { Value = 1 }
+    private enum LongEnum : long { Value = 1 }
+    private enum ULongEnum : long { Value = 1 }
+
+    private struct UserDefinedStruct
+    {
+      public int Value;
+    }
+    #endregion
+
+    [TestCase(typeof(string))]
+    [TestCase(typeof(object))]
+    public void IsReadWriteAtomic_ReferenceTypes_ReturnsTrue(Type type)
+    {
+      Assert.IsTrue(IsReadWriteAtomic(type));
+    }
+
+    [TestCase(typeof(bool))]
+    [TestCase(typeof(byte))]
+    [TestCase(typeof(sbyte))]
+    [TestCase(typeof(char))]
+    [TestCase(typeof(short))]
+    [TestCase(typeof(ushort))]
+    [TestCase(typeof(int))]
+    [TestCase(typeof(uint))]
+    [TestCase(typeof(float))]
+    [TestCase(typeof(IntPtr))]
+    [TestCase(typeof(UIntPtr))]
+    public void IsReadWriteAtomic_PrimitiveTypes_ReturnsTrue(Type type)
+    {
+      Assert.IsTrue(IsReadWriteAtomic(type));
+    }
+
+    [TestCase(typeof(long))]
+    [TestCase(typeof(ulong))]
+    [TestCase(typeof(double))]
+    public void IsReadWriteAtomic_LargePrimitiveTypes_DependsOnArchitecture(Type type)
+    {
+      Assert.AreEqual(IsReadWriteAtomic(type), IntPtr.Size == 8);
+    }
+
+    [TestCase(typeof(ByteEnum))]
+    [TestCase(typeof(IntEnum))]
+    [TestCase(typeof(UIntEnum))]
+    public void IsReadWriteAtomic_Enums_ReturnsTrue(Type type)
+    {
+      Assert.IsTrue(IsReadWriteAtomic(type));
+    }
+
+    [TestCase(typeof(LongEnum))]
+    [TestCase(typeof(ULongEnum))]
+    public void IsReadWriteAtomic_LargeEnums_DependsOnArchitecture(Type type)
+    {
+      Assert.AreEqual(IsReadWriteAtomic(type), IntPtr.Size == 8);
+    }
+
+    [TestCase(typeof(DateTime))]
+    [TestCase(typeof(decimal))]
+    [TestCase(typeof(UserDefinedStruct))]
+    public void IsReadWriteAtomic_UserDefinedStructs_ReturnsFalse(Type type)
+    {
+      Assert.IsFalse(IsReadWriteAtomic(type));
+    }
+
+    private static bool IsReadWriteAtomic(Type type)
+    {
+      var canon = typeof(Memory).GetMethod(nameof(Memory.IsReadWriteAtomic));
+      var generic = canon!.MakeGenericMethod(type);
+      return (bool)generic.Invoke(null,null);
+    }
+  }
+}