fix: RxObject memory allocation for change delays (#8)

RLittlesII · web-flow · commit 67df0ce351d4 · 2021-08-29T08:16:18.000Z
diff --git a/src/ReactiveMarbles.Mvvm.Benchmarks/README.MD b/src/ReactiveMarbles.Mvvm.Benchmarks/README.MD
@@ -1,21 +1,22 @@
 ## RxObject
 
-|             Method | CreateNumber |           Mean |         Error |        StdDev |         Median |    Gen 0 |    Gen 1 | Gen 2 | Allocated |
-|------------------- |------------- |---------------:|--------------:|--------------:|---------------:|---------:|---------:|------:|----------:|
-|   RxObjectCreation |            1 |       564.8 ns |      10.96 ns |      14.25 ns |       566.5 ns |   0.2022 |   0.0010 |     - |      2 KB |
-| RxObjectWithChange |            1 |       647.4 ns |       9.88 ns |       8.76 ns |       647.0 ns |   0.2079 |   0.0010 |     - |      2 KB |
-|   RxObjectCreation |          100 |    50,249.3 ns |     917.04 ns |     857.80 ns |    50,184.9 ns |  18.5547 |   5.7983 |     - |    152 KB |
-| RxObjectWithChange |          100 |    56,502.8 ns |     911.23 ns |     852.37 ns |    56,459.0 ns |  19.1040 |   5.1270 |     - |    156 KB |
-|   RxObjectCreation |         4000 | 5,593,739.2 ns |  69,053.41 ns |  64,592.60 ns | 5,600,920.0 ns | 742.1875 | 367.1875 |     - |  6,063 KB |
-| RxObjectWithChange |         4000 | 6,234,991.0 ns | 122,895.87 ns | 224,722.04 ns | 6,133,371.3 ns | 757.8125 | 375.0000 |     - |  6,250 KB |
+|             Method | CreateNumber |           Mean |        Error |       StdDev |    Gen 0 |    Gen 1 | Gen 2 |   Allocated |
+|------------------- |------------- |---------------:|-------------:|-------------:|---------:|---------:|------:|------------:|
+|   RxObjectCreation |            1 |       206.5 ns |      2.15 ns |      2.01 ns |   0.0782 |        - |     - |       656 B |
+| RxObjectWithChange |            1 |       280.2 ns |      5.19 ns |      4.85 ns |   0.0839 |        - |     - |       704 B |
+|   RxObjectCreation |          100 |    12,782.4 ns |    212.66 ns |    188.52 ns |   6.1340 |   0.9308 |     - |    51,344 B |
+| RxObjectWithChange |          100 |    17,117.2 ns |    318.94 ns |    282.74 ns |   6.6833 |   0.9155 |     - |    56,144 B |
+|   RxObjectCreation |         4000 |   904,746.9 ns | 15,505.56 ns | 13,745.27 ns | 244.1406 | 112.3047 |     - | 2,048,154 B |
+| RxObjectWithChange |         4000 | 1,269,486.4 ns | 24,327.69 ns | 22,756.13 ns | 267.5781 | 132.8125 |     - | 2,240,152 B |
 
 
 ## ReactiveUI ReactiveObject
+
 |                   Method | CreateNumber |            Mean |         Error |        StdDev |    Gen 0 |    Gen 1 |    Gen 2 |   Allocated |
 |------------------------- |------------- |----------------:|--------------:|--------------:|---------:|---------:|---------:|------------:|
-|   ReactiveObjectCreation |            1 |        242.4 ns |       4.57 ns |       6.26 ns |   0.1068 |        - |        - |       896 B |
-| ReactiveObjectWithChange |            1 |      1,536.5 ns |      29.92 ns |      42.91 ns |   0.2575 |   0.0057 |   0.0019 |     2,157 B |
-|   ReactiveObjectCreation |          100 |     16,884.4 ns |     335.55 ns |     560.64 ns |   9.0027 |   1.7090 |        - |    75,344 B |
-| ReactiveObjectWithChange |          100 |    133,628.0 ns |   2,625.84 ns |   4,598.94 ns |  24.4141 |   1.7090 |   0.4883 |   203,267 B |
-|   ReactiveObjectCreation |         4000 |  1,579,677.4 ns |  30,944.54 ns |  34,394.78 ns | 359.3750 | 179.6875 |        - | 3,008,163 B |
-| ReactiveObjectWithChange |         4000 | 11,459,659.3 ns | 227,605.87 ns | 212,902.68 ns | 953.1250 | 468.7500 | 218.7500 | 8,091,142 B |
+|   ReactiveObjectCreation |            1 |        238.8 ns |       3.27 ns |       3.06 ns |   0.1068 |        - |        - |       896 B |
+| ReactiveObjectWithChange |            1 |      1,530.3 ns |      30.56 ns |      40.79 ns |   0.2575 |   0.0057 |   0.0019 |     2,157 B |
+|   ReactiveObjectCreation |          100 |     15,796.5 ns |     314.80 ns |     398.12 ns |   9.0027 |   1.7090 |        - |    75,344 B |
+| ReactiveObjectWithChange |          100 |    132,292.6 ns |   2,553.90 ns |   3,580.21 ns |  24.1699 |   1.9531 |   0.7324 |   203,267 B |
+|   ReactiveObjectCreation |         4000 |  1,530,059.8 ns |  26,466.29 ns |  24,756.59 ns | 359.3750 | 179.6875 |        - | 3,008,145 B |
+| ReactiveObjectWithChange |         4000 | 11,304,993.4 ns | 146,890.54 ns | 137,401.50 ns | 953.1250 | 468.7500 | 218.7500 | 8,091,139 B |
diff --git a/src/ReactiveMarbles.Mvvm/RxObject.cs b/src/ReactiveMarbles.Mvvm/RxObject.cs
@@ -5,6 +5,7 @@
 using System;
 using System.Collections.Generic;
 using System.ComponentModel;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Reactive.Disposables;
 using System.Reactive.Linq;
@@ -22,11 +23,8 @@ namespace ReactiveMarbles.Core
     /// </summary>
     public class RxObject : IRxObject
     {
-        private readonly SourceList<RxPropertyChangedEventArgs<IRxObject>> _propertyChangedEvents = new();
-        private readonly SourceList<RxPropertyChangingEventArgs<IRxObject>> _propertyChangingEvents = new();
         private readonly Lazy<Subject<Exception>> _thrownExceptions = new(() => new Subject<Exception>(), LazyThreadSafetyMode.PublicationOnly);
-        private long _changeNotificationsDelayed;
-        private long _changeNotificationsSuppressed;
+        private readonly Lazy<Notifications> _notification = new(() => new Notifications());
 
         /// <summary>
         /// Initializes a new instance of the <see cref="RxObject"/> class.
@@ -68,39 +66,39 @@ void Handler(object sender, PropertyChangingEventArgs args) =>
         public IObservable<RxPropertyChangedEventArgs<IRxObject>> Changed { get; }
 
         /// <inheritdoc/>
-        public bool AreChangeNotificationsEnabled() => Interlocked.Read(ref _changeNotificationsSuppressed) == 0;
+        public bool AreChangeNotificationsEnabled() => !_notification.IsValueCreated || Interlocked.Read(ref _notification.Value.ChangeNotificationsSuppressed) == 0;
 
         /// <inheritdoc/>
-        public bool AreChangeNotificationsDelayed() => Interlocked.Read(ref _changeNotificationsDelayed) > 0;
+        public bool AreChangeNotificationsDelayed() => _notification.IsValueCreated && Interlocked.Read(ref _notification.Value.ChangeNotificationsDelayed) > 0;
 
         /// <inheritdoc/>
         public IDisposable SuppressChangeNotifications()
         {
-            Interlocked.Increment(ref _changeNotificationsSuppressed);
-            return Disposable.Create(() => Interlocked.Decrement(ref _changeNotificationsSuppressed));
+            Interlocked.Increment(ref _notification.Value.ChangeNotificationsSuppressed);
+            return Disposable.Create(() => Interlocked.Decrement(ref _notification.Value.ChangeNotificationsSuppressed));
         }
 
         /// <inheritdoc/>
         public IDisposable DelayChangeNotifications()
         {
-            Interlocked.Increment(ref _changeNotificationsDelayed);
+            Interlocked.Increment(ref _notification.Value.ChangeNotificationsDelayed);
 
             return Disposable.Create(() =>
             {
-                if (Interlocked.Decrement(ref _changeNotificationsDelayed) == 0)
+                if (Interlocked.Decrement(ref _notification.Value.ChangeNotificationsDelayed) == 0)
                 {
-                    foreach (var distinctEvent in DistinctEvents(_propertyChangingEvents.Items.ToList()))
+                    foreach (var distinctEvent in DistinctEvents(_notification.Value.PropertyChangingEvents.Items.ToList()))
                     {
                         PropertyChanging?.Invoke(this, new PropertyChangingEventArgs(distinctEvent.PropertyName));
                     }
 
-                    foreach (var distinctEvent in DistinctEvents(_propertyChangedEvents.Items.ToList()))
+                    foreach (var distinctEvent in DistinctEvents(_notification.Value.PropertyChangedEvents.Items.ToList()))
                     {
                         PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(distinctEvent.PropertyName));
                     }
 
-                    _propertyChangedEvents.Clear();
-                    _propertyChangingEvents.Clear();
+                    _notification.Value.PropertyChangingEvents.Clear();
+                    _notification.Value.PropertyChangedEvents.Clear();
                 }
             });
         }
@@ -118,7 +116,7 @@ protected void RaisePropertyChanging(PropertyChangingEventArgs args)
 
             if (AreChangeNotificationsDelayed())
             {
-                _propertyChangingEvents.Add(new RxPropertyChangingEventArgs<IRxObject>(args.PropertyName, this));
+                _notification.Value.PropertyChangingEvents.Add(new RxPropertyChangingEventArgs<IRxObject>(args.PropertyName, this));
                 return;
             }
 
@@ -148,7 +146,7 @@ protected void RaisePropertyChanged(PropertyChangedEventArgs args)
 
             if (AreChangeNotificationsDelayed())
             {
-                _propertyChangedEvents.Add(new RxPropertyChangedEventArgs<IRxObject>(args.PropertyName, this));
+                _notification.Value.PropertyChangedEvents.Add(new RxPropertyChangedEventArgs<IRxObject>(args.PropertyName, this));
                 return;
             }
 
@@ -234,5 +232,14 @@ private static IEnumerable<TEventArgs> DistinctEvents<TEventArgs>(IList<TEventAr
             // Stack enumerates in LIFO order
             return uniqueEvents;
         }
+
+        [SuppressMessage("StyleCop", "SA1401", Justification = "Deliberate use of private field")]
+        private class Notifications
+        {
+            public readonly SourceList<RxPropertyChangedEventArgs<IRxObject>> PropertyChangedEvents = new();
+            public readonly SourceList<RxPropertyChangingEventArgs<IRxObject>> PropertyChangingEvents = new();
+            public long ChangeNotificationsDelayed = 0;
+            public long ChangeNotificationsSuppressed = 0;
+        }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@`
`5`	`5`	`using System;`
`6`	`6`	`using System.Collections.Generic;`
`7`	`7`	`using System.ComponentModel;`
	`8`	`+using System.Diagnostics.CodeAnalysis;`
`8`	`9`	`using System.Linq;`
`9`	`10`	`using System.Reactive.Disposables;`
`10`	`11`	`using System.Reactive.Linq;`
`@@ -22,11 +23,8 @@ namespace ReactiveMarbles.Core`
`22`	`23`	`/// </summary>`
`23`	`24`	`public class RxObject : IRxObject`
`24`	`25`	`{`
`25`		`- private readonly SourceList<RxPropertyChangedEventArgs<IRxObject>> _propertyChangedEvents = new();`
`26`		`- private readonly SourceList<RxPropertyChangingEventArgs<IRxObject>> _propertyChangingEvents = new();`
`27`	`26`	`private readonly Lazy<Subject<Exception>> _thrownExceptions = new(() => new Subject<Exception>(), LazyThreadSafetyMode.PublicationOnly);`
`28`		`- private long _changeNotificationsDelayed;`
`29`		`- private long _changeNotificationsSuppressed;`
	`27`	`+ private readonly Lazy<Notifications> _notification = new(() => new Notifications());`
`30`	`28`
`31`	`29`	`/// <summary>`
`32`	`30`	`/// Initializes a new instance of the <see cref="RxObject"/> class.`
`@@ -68,39 +66,39 @@ void Handler(object sender, PropertyChangingEventArgs args) =>`
`68`	`66`	`public IObservable<RxPropertyChangedEventArgs<IRxObject>> Changed { get; }`
`69`	`67`
`70`	`68`	`/// <inheritdoc/>`
`71`		`- public bool AreChangeNotificationsEnabled() => Interlocked.Read(ref _changeNotificationsSuppressed) == 0;`
	`69`	`+ public bool AreChangeNotificationsEnabled() => !_notification.IsValueCreated \|\| Interlocked.Read(ref _notification.Value.ChangeNotificationsSuppressed) == 0;`
`72`	`70`
`73`	`71`	`/// <inheritdoc/>`
`74`		`- public bool AreChangeNotificationsDelayed() => Interlocked.Read(ref _changeNotificationsDelayed) > 0;`
	`72`	`+ public bool AreChangeNotificationsDelayed() => _notification.IsValueCreated && Interlocked.Read(ref _notification.Value.ChangeNotificationsDelayed) > 0;`
`75`	`73`
`76`	`74`	`/// <inheritdoc/>`
`77`	`75`	`public IDisposable SuppressChangeNotifications()`
`78`	`76`	`{`
`79`		`- Interlocked.Increment(ref _changeNotificationsSuppressed);`
`80`		`- return Disposable.Create(() => Interlocked.Decrement(ref _changeNotificationsSuppressed));`
	`77`	`+ Interlocked.Increment(ref _notification.Value.ChangeNotificationsSuppressed);`
	`78`	`+ return Disposable.Create(() => Interlocked.Decrement(ref _notification.Value.ChangeNotificationsSuppressed));`
`81`	`79`	`}`
`82`	`80`
`83`	`81`	`/// <inheritdoc/>`
`84`	`82`	`public IDisposable DelayChangeNotifications()`
`85`	`83`	`{`
`86`		`- Interlocked.Increment(ref _changeNotificationsDelayed);`
	`84`	`+ Interlocked.Increment(ref _notification.Value.ChangeNotificationsDelayed);`
`87`	`85`
`88`	`86`	`return Disposable.Create(() =>`
`89`	`87`	`{`
`90`		`- if (Interlocked.Decrement(ref _changeNotificationsDelayed) == 0)`
	`88`	`+ if (Interlocked.Decrement(ref _notification.Value.ChangeNotificationsDelayed) == 0)`
`91`	`89`	`{`
`92`		`- foreach (var distinctEvent in DistinctEvents(_propertyChangingEvents.Items.ToList()))`
	`90`	`+ foreach (var distinctEvent in DistinctEvents(_notification.Value.PropertyChangingEvents.Items.ToList()))`
`93`	`91`	`{`
`94`	`92`	`PropertyChanging?.Invoke(this, new PropertyChangingEventArgs(distinctEvent.PropertyName));`
`95`	`93`	`}`
`96`	`94`
`97`		`- foreach (var distinctEvent in DistinctEvents(_propertyChangedEvents.Items.ToList()))`
	`95`	`+ foreach (var distinctEvent in DistinctEvents(_notification.Value.PropertyChangedEvents.Items.ToList()))`
`98`	`96`	`{`
`99`	`97`	`PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(distinctEvent.PropertyName));`
`100`	`98`	`}`
`101`	`99`
`102`		`- _propertyChangedEvents.Clear();`
`103`		`- _propertyChangingEvents.Clear();`
	`100`	`+ _notification.Value.PropertyChangingEvents.Clear();`
	`101`	`+ _notification.Value.PropertyChangedEvents.Clear();`
`104`	`102`	`}`
`105`	`103`	`});`
`106`	`104`	`}`
`@@ -118,7 +116,7 @@ protected void RaisePropertyChanging(PropertyChangingEventArgs args)`
`118`	`116`
`119`	`117`	`if (AreChangeNotificationsDelayed())`
`120`	`118`	`{`
`121`		`- _propertyChangingEvents.Add(new RxPropertyChangingEventArgs<IRxObject>(args.PropertyName, this));`
	`119`	`+ _notification.Value.PropertyChangingEvents.Add(new RxPropertyChangingEventArgs<IRxObject>(args.PropertyName, this));`
`122`	`120`	`return;`
`123`	`121`	`}`
`124`	`122`
`@@ -148,7 +146,7 @@ protected void RaisePropertyChanged(PropertyChangedEventArgs args)`
`148`	`146`
`149`	`147`	`if (AreChangeNotificationsDelayed())`
`150`	`148`	`{`
`151`		`- _propertyChangedEvents.Add(new RxPropertyChangedEventArgs<IRxObject>(args.PropertyName, this));`
	`149`	`+ _notification.Value.PropertyChangedEvents.Add(new RxPropertyChangedEventArgs<IRxObject>(args.PropertyName, this));`
`152`	`150`	`return;`
`153`	`151`	`}`
`154`	`152`
`@@ -234,5 +232,14 @@ private static IEnumerable<TEventArgs> DistinctEvents<TEventArgs>(IList<TEventAr`
`234`	`232`	`// Stack enumerates in LIFO order`
`235`	`233`	`return uniqueEvents;`
`236`	`234`	`}`
	`235`	`+`
	`236`	`+ [SuppressMessage("StyleCop", "SA1401", Justification = "Deliberate use of private field")]`
	`237`	`+ private class Notifications`
	`238`	`+ {`
	`239`	`+ public readonly SourceList<RxPropertyChangedEventArgs<IRxObject>> PropertyChangedEvents = new();`
	`240`	`+ public readonly SourceList<RxPropertyChangingEventArgs<IRxObject>> PropertyChangingEvents = new();`
	`241`	`+ public long ChangeNotificationsDelayed = 0;`
	`242`	`+ public long ChangeNotificationsSuppressed = 0;`
	`243`	`+ }`
`237`	`244`	`}`
`238`	`245`	`}`