fix DisposableBuilder

Cysharp · Dec 27, 2023 · 935fcb4 · 935fcb4
1 parent 74c3645
commit 935fcb4
Show file tree

Hide file tree

Showing 4 changed files with 154 additions and 19 deletions.
diff --git a/src/R3/Disposable.cs b/src/R3/Disposable.cs
@@ -32,7 +32,7 @@ public static CancellationTokenRegistration RegisterTo(this IDisposable disposab
             return default;
         }
 
-        return cancellationToken.Register(state =>
+        return cancellationToken.UnsafeRegister(state =>
         {
             var d = ((IDisposable)state!);
             d.Dispose();
@@ -160,14 +160,14 @@ public static IDisposable Combine(params IDisposable[] disposables)
     }
 }
 
-internal class EmptyDisposable : IDisposable
+internal sealed class EmptyDisposable : IDisposable
 {
     public void Dispose()
     {
     }
 }
 
-internal class CombinedDisposable2(IDisposable disposable1, IDisposable disposable2) : IDisposable
+internal sealed class CombinedDisposable2(IDisposable disposable1, IDisposable disposable2) : IDisposable
 {
     public void Dispose()
     {
@@ -176,7 +176,7 @@ public void Dispose()
     }
 }
 
-internal class CombinedDisposable3(IDisposable disposable1, IDisposable disposable2, IDisposable disposable3) : IDisposable
+internal sealed class CombinedDisposable3(IDisposable disposable1, IDisposable disposable2, IDisposable disposable3) : IDisposable
 {
     public void Dispose()
     {
@@ -186,7 +186,7 @@ public void Dispose()
     }
 }
 
-internal class CombinedDisposable4(
+internal sealed class CombinedDisposable4(
     IDisposable disposable1,
     IDisposable disposable2,
     IDisposable disposable3,
@@ -202,7 +202,7 @@ public void Dispose()
 }
 
 
-internal class CombinedDisposable5(
+internal sealed class CombinedDisposable5(
     IDisposable disposable1,
     IDisposable disposable2,
     IDisposable disposable3,
@@ -220,7 +220,7 @@ public void Dispose()
 }
 
 
-internal class CombinedDisposable6(
+internal sealed class CombinedDisposable6(
     IDisposable disposable1,
     IDisposable disposable2,
     IDisposable disposable3,
@@ -239,7 +239,7 @@ public void Dispose()
     }
 }
 
-internal class CombinedDisposable7(
+internal sealed class CombinedDisposable7(
     IDisposable disposable1,
     IDisposable disposable2,
     IDisposable disposable3,
@@ -260,7 +260,7 @@ public void Dispose()
     }
 }
 
-internal class CombinedDisposable8(
+internal sealed class CombinedDisposable8(
     IDisposable disposable1,
     IDisposable disposable2,
     IDisposable disposable3,
@@ -353,6 +353,7 @@ void AddToArray(IDisposable disposable)
         if (count == 8)
         {
             var newDisposables = ArrayPool<IDisposable>.Shared.Rent(16);
+            newDisposables[8] = disposable; // JIT optimize
             newDisposables[0] = disposable1!;
             newDisposables[1] = disposable2!;
             newDisposables[2] = disposable3!;
@@ -362,16 +363,18 @@ void AddToArray(IDisposable disposable)
             newDisposables[6] = disposable7!;
             newDisposables[7] = disposable8!;
             disposable1 = disposable2 = disposable3 = disposable4 = disposable5 = disposable6 = disposable7 = disposable8 = null;
-
-            newDisposables[8] = disposable;
+            disposables = newDisposables; // assign
         }
         else
         {
-            var newDisposables = ArrayPool<IDisposable>.Shared.Rent(disposables!.Length * 2);
-            Array.Copy(disposables, newDisposables, disposables.Length);
-            ArrayPool<IDisposable>.Shared.Return(disposables, clearArray: true);
-            newDisposables[count] = disposable;
-            disposables = newDisposables;
+            if (disposables!.Length == count)
+            {
+                var newDisposables = ArrayPool<IDisposable>.Shared.Rent(count * 2);
+                Array.Copy(disposables, newDisposables, disposables.Length);
+                ArrayPool<IDisposable>.Shared.Return(disposables, clearArray: true);
+                disposables = newDisposables;
+            }
+            disposables[count] = disposable;
         }
     }
 
@@ -452,7 +455,7 @@ public IDisposable Build()
                 );
                 break;
             default:
-                result = new CombinedDisposable(disposables!);
+                result = new CombinedDisposable(disposables!.AsSpan(0, count).ToArray());
                 break;
         }
 

diff --git a/src/R3/Factories/Amb.cs b/src/R3/Factories/Amb.cs
@@ -0,0 +1,36 @@
+namespace R3;
+
+public static partial class Observable
+{
+    public static IObservable<T> Amb<T>(params IObservable<T>[] sources)
+    {
+        throw new NotImplementedException();
+    }
+
+    public static IObservable<T> Amb<T>(IEnumerable<IObservable<T>> sources)
+    {
+        throw new NotImplementedException();
+    }
+}
+
+internal sealed class Amb<T>(IEnumerable<IObservable<T>> sources) : Observable<T>
+{
+    protected override IDisposable SubscribeCore(Observer<T> observer)
+    {
+        //new CompositeDiposableBuilder
+        // Disposable.CreateBuilder();
+        throw new NotImplementedException();
+        //if (sources.TryGetNonEnumeratedCount(out var count))
+        //{
+
+
+        //}
+        //else
+        //{
+
+        //}
+        // throw new NotImplementedException();
+    }
+
+
+}
diff --git a/src/R3/Operators/_Operators.cs b/src/R3/Operators/_Operators.cs
@@ -1,4 +1,5 @@
-namespace R3;
+
+namespace R3;
 
 public static partial class ObservableExtensions
 {
@@ -18,7 +19,7 @@ public static partial class ObservableExtensions
     // Observe
 
     // Rx Merging:
-    //CombineLatest, Merge, Zip, WithLatestFrom, ZipLatest, Switch, MostRecent
+    //CombineLatest, Merge, Zip, WithLatestFrom, ZipLatest, Switch
 
     // Standard Query:
     // Distinct, DistinctUntilChanged, Scan

diff --git a/tests/R3.Tests/DisposableBuilderTest.cs b/tests/R3.Tests/DisposableBuilderTest.cs
@@ -0,0 +1,95 @@
+namespace R3.Tests;
+
+public class DisposableBuilderTest
+{
+    // 1~8 = Combined, 9~ array
+    [Fact]
+    public void Combined()
+    {
+        // combined check
+        for (int i = 1; i <= 8; i++)
+        {
+            var l = new List<int>();
+            using var builder = Disposable.CreateBuilder();
+            for (int j = 0; j < i; j++)
+            {
+                builder.Add(Disposable.Create(() => l.Add(j + 1)));
+            }
+
+            var disposable = builder.Build();
+
+            if (i == 1)
+            {
+                disposable.GetType().Name.Should().Be("AnonymousDisposable");
+            }
+            else
+            {
+                disposable.GetType().Name.Should().StartWith("CombinedDisposable");
+            }
+
+            l.Should().BeEmpty();
+
+            disposable.Dispose();
+
+            l.Should().HaveCount(i);
+        }
+    }
+
+    [Fact]
+    public void Array()
+    {
+        var l = new List<int>();
+        using var builder = Disposable.CreateBuilder();
+        for (int i = 1; i <= 8; i++)
+        {
+            var v = i;
+            builder.Add(Disposable.Create(() => l.Add(v)));
+        }
+
+        // array
+        builder.Add(Disposable.Create(() => l.Add(9)));
+        builder.Add(Disposable.Create(() => l.Add(10)));
+        builder.Add(Disposable.Create(() => l.Add(11)));
+        builder.Add(Disposable.Create(() => l.Add(12)));
+        builder.Add(Disposable.Create(() => l.Add(13)));
+        builder.Add(Disposable.Create(() => l.Add(14)));
+        builder.Add(Disposable.Create(() => l.Add(15)));
+        builder.Add(Disposable.Create(() => l.Add(16)));
+        // grow
+        builder.Add(Disposable.Create(() => l.Add(17)));
+        builder.Add(Disposable.Create(() => l.Add(18)));
+        builder.Add(Disposable.Create(() => l.Add(19)));
+        builder.Add(Disposable.Create(() => l.Add(20)));
+
+        var disposable = builder.Build();
+
+        disposable.GetType().Name.Should().Be("CombinedDisposable");
+
+        l.Should().BeEmpty();
+
+        disposable.Dispose();
+
+        l.Should().Equal([
+            1,
+            2,
+            3,
+            4,
+            5,
+            6,
+            7,
+            8,
+            9,
+            10,
+            11,
+            12,
+            13,
+            14,
+            15,
+            16,
+            17,
+            18,
+            19,
+            20]);
+    }
+
+}