Skip to content

Commit 85aa092

Browse files
authored
Removed some useless locks on signal property change observable
1 parent 331f2d3 commit 85aa092

File tree

9 files changed

+58
-156
lines changed

9 files changed

+58
-156
lines changed

SignalsDotnet/SignalsDotnet/Helpers/Optional.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Diagnostics.CodeAnalysis;
2+
using System.Runtime.CompilerServices;
23

34
namespace SignalsDotnet.Helpers;
45

@@ -14,9 +15,11 @@ public readonly struct Optional<T>
1415

1516
public static class OptionalExtensions
1617
{
18+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
1719
public static bool TryGetValue<T>(this Optional<T> @this, [NotNullWhen(true)] out T? value)
1820
{
19-
value = @this.HasValue ? @this.Value : default;
20-
return @this.HasValue;
21+
var hasValue = @this.HasValue;
22+
value = hasValue ? @this.Value : default;
23+
return hasValue;
2124
}
2225
}

SignalsDotnet/SignalsDotnet/Internals/ComputedObservable.cs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
namespace SignalsDotnet.Internals;
66

7-
internal class ComputedObservable<T> : Observable<T>
7+
internal sealed class ComputedObservable<T> : Observable<T>
88
{
99
readonly Func<CancellationToken, ValueTask<T>> _func;
1010
readonly Func<Optional<T>> _fallbackValue;
@@ -24,7 +24,7 @@ public ComputedObservable(Func<CancellationToken, ValueTask<T>> func,
2424

2525
protected override IDisposable SubscribeCore(Observer<T> observer) => new Subscription(this, observer);
2626

27-
class Subscription : IDisposable
27+
sealed class Subscription : IDisposable
2828
{
2929
readonly ComputedObservable<T> _observable;
3030
readonly Observer<T> _observer;
@@ -45,9 +45,9 @@ async void WatchSignalsChanges()
4545
try
4646
{
4747
var token = _disposed.Token;
48-
while (!token.IsCancellationRequested)
48+
do
4949
{
50-
var result = await ComputeResult(_disposed.Token);
50+
var result = await ComputeResult(token);
5151
if (token.IsCancellationRequested) return;
5252

5353
if (result.ResultOptional.TryGetValue(out var propertyValue))
@@ -58,7 +58,7 @@ async void WatchSignalsChanges()
5858
await using var _ = token.Register(static x => ((SyncCompletionSource)x!).SetCompleted(Unit.Default), result.SignalChangedAwaitable);
5959
await result.SignalChangedAwaitable;
6060
_disconnectSubscription.Dispose();
61-
}
61+
} while (!token.IsCancellationRequested);
6262
}
6363
finally
6464
{
@@ -74,8 +74,7 @@ async void WatchSignalsChanges()
7474

7575
async ValueTask<ComputationResult> ComputeResult(CancellationToken cancellationToken)
7676
{
77-
var referenceEquality = ReferenceEqualityComparer.Instance;
78-
HashSet<IReadOnlySignal> signalRequested = new(referenceEquality);
77+
var signalsRequested = new HashSet<IReadOnlySignal>(ReferenceEqualityComparer.Instance);
7978
Optional<T> result;
8079

8180
_disconnectSubscription = new();
@@ -97,7 +96,7 @@ async ValueTask<ComputationResult> ComputeResult(CancellationToken cancellationT
9796
var signalRequestedSubscription = Signal.SignalsRequested()
9897
.Subscribe(signal =>
9998
{
100-
if (!signalRequested.Add(signal)) return;
99+
if (!signalsRequested.Add(signal)) return;
101100

102101
signal.FutureValues.Subscribe(_ =>
103102
{
@@ -159,8 +158,8 @@ public void Dispose()
159158
}
160159
}
161160

162-
record struct ComputationResult(SyncCompletionSource SignalChangedAwaitable, Optional<T> ResultOptional);
163-
class SyncCompletionSource : INotifyCompletion
161+
readonly record struct ComputationResult(SyncCompletionSource SignalChangedAwaitable, Optional<T> ResultOptional);
162+
sealed class SyncCompletionSource : INotifyCompletion
164163
{
165164
Action? _continuation;
166165
public SyncCompletionSource GetAwaiter() => this;
@@ -184,7 +183,7 @@ public void GetResult() {}
184183
}
185184
}
186185

187-
internal class ActionStub
186+
internal sealed class ActionStub
188187
{
189188
public static readonly Action Nop = () => { };
190189
}

SignalsDotnet/SignalsDotnet/Internals/ComputedSignalrFactory/DefaultComputedSignalFactory.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
namespace SignalsDotnet.Internals.ComputedSignalrFactory;
66

7-
internal class DefaultComputedSignalFactory : IComputedSignalFactory
7+
internal sealed class DefaultComputedSignalFactory : IComputedSignalFactory
88
{
99
public IReadOnlySignal<T> Computed<T>(Func<T> func, Func<Optional<T>> fallbackValue, ReadonlySignalConfigurationDelegate<T?>? configuration = null)
1010
{

SignalsDotnet/SignalsDotnet/Internals/ComputedSignalrFactory/OnErrorComputedSignalFactoryDecorator.cs

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,16 @@
55

66
namespace SignalsDotnet.Internals.ComputedSignalrFactory;
77

8-
internal class OnErrorComputedSignalFactoryDecorator : IComputedSignalFactory
8+
internal sealed class OnErrorComputedSignalFactoryDecorator(IComputedSignalFactory parent, bool ignoreOperationCancelled, Action<Exception> onException) : IComputedSignalFactory
99
{
10-
readonly IComputedSignalFactory _parent;
11-
readonly bool _ignoreOperationCancelled;
12-
readonly Action<Exception> _onException;
13-
14-
public OnErrorComputedSignalFactoryDecorator(IComputedSignalFactory parent, bool ignoreOperationCancelled, Action<Exception> onException)
15-
{
16-
_parent = parent;
17-
_ignoreOperationCancelled = ignoreOperationCancelled;
18-
_onException = onException;
19-
}
20-
2110
public IReadOnlySignal<T> Computed<T>(Func<T> func, Func<Optional<T>> fallbackValue, ReadonlySignalConfigurationDelegate<T?>? configuration = null)
2211
{
2312
return ComputedObservable(func, fallbackValue).ToSignal(configuration!);
2413
}
2514

2615
public Observable<T> ComputedObservable<T>(Func<T> func, Func<Optional<T>> fallbackValue)
2716
{
28-
return _parent.ComputedObservable(() =>
17+
return parent.ComputedObservable(() =>
2918
{
3019
try
3120
{
@@ -47,7 +36,7 @@ public IAsyncReadOnlySignal<T> AsyncComputed<T>(Func<CancellationToken, ValueTas
4736

4837
public Observable<T> AsyncComputedObservable<T>(Func<CancellationToken, ValueTask<T>> func, T startValue, Func<Optional<T>> fallbackValue, ConcurrentChangeStrategy concurrentChangeStrategy = default)
4938
{
50-
return _parent.AsyncComputedObservable(async token =>
39+
return parent.AsyncComputedObservable(async token =>
5140
{
5241
try
5342
{
@@ -63,7 +52,7 @@ public Observable<T> AsyncComputedObservable<T>(Func<CancellationToken, ValueTas
6352

6453
public ISignal<T> Linked<T>(Func<T> func, Func<Optional<T>> fallbackValue, ReadonlySignalConfigurationDelegate<T?>? configuration = null)
6554
{
66-
return _parent.Linked(() =>
55+
return parent.Linked(() =>
6756
{
6857
try
6958
{
@@ -83,7 +72,7 @@ public IAsyncSignal<T> AsyncLinked<T>(Func<CancellationToken, ValueTask<T>> func
8372
ConcurrentChangeStrategy concurrentChangeStrategy = default,
8473
ReadonlySignalConfigurationDelegate<T>? configuration = null)
8574
{
86-
return _parent.AsyncLinked(async token =>
75+
return parent.AsyncLinked(async token =>
8776
{
8877
try
8978
{
@@ -99,7 +88,7 @@ public IAsyncSignal<T> AsyncLinked<T>(Func<CancellationToken, ValueTask<T>> func
9988

10089
public Effect Effect(Action onChange, TimeProvider? scheduler = null)
10190
{
102-
return _parent.Effect(() =>
91+
return parent.Effect(() =>
10392
{
10493
try
10594
{
@@ -115,7 +104,7 @@ public Effect Effect(Action onChange, TimeProvider? scheduler = null)
115104

116105
public Effect AsyncEffect(Func<CancellationToken, ValueTask> onChange, ConcurrentChangeStrategy concurrentChangeStrategy = default, TimeProvider? scheduler = null)
117106
{
118-
return _parent.AsyncEffect(async token =>
107+
return parent.AsyncEffect(async token =>
119108
{
120109
try
121110
{
@@ -131,11 +120,14 @@ public Effect AsyncEffect(Func<CancellationToken, ValueTask> onChange, Concurren
131120

132121
void NotifyException(Exception e)
133122
{
134-
if (_ignoreOperationCancelled && e is OperationCanceledException)
123+
if (ignoreOperationCancelled && e is OperationCanceledException)
135124
{
136125
return;
137126
}
138127

139-
Signal.Untracked(() => _onException(e));
128+
using (Signal.UntrackedScope())
129+
{
130+
onException(e);
131+
}
140132
}
141133
}
Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,17 @@
11
namespace SignalsDotnet.Internals.Helpers;
22

3-
internal class KeyEqualityComparer<T, TDestination> : IEqualityComparer<T> where TDestination : notnull
3+
internal sealed class KeyEqualityComparer<T, TDestination>(Func<T?, TDestination> keyExtractor) : IEqualityComparer<T>
4+
where TDestination : notnull
45
{
5-
readonly Func<T?, TDestination> _keyExtractor;
66
readonly EqualityComparer<TDestination> _equalityComparer = EqualityComparer<TDestination>.Default;
77

8-
public KeyEqualityComparer(Func<T?, TDestination> keyExtractor)
9-
{
10-
_keyExtractor = keyExtractor;
11-
}
12-
138
public bool Equals(T? x, T? y)
149
{
15-
return _equalityComparer.Equals(_keyExtractor(x), _keyExtractor(y));
10+
return _equalityComparer.Equals(keyExtractor(x), keyExtractor(y));
1611
}
1712

1813
public int GetHashCode(T? obj)
1914
{
20-
return _equalityComparer.GetHashCode(_keyExtractor(obj));
15+
return _equalityComparer.GetHashCode(keyExtractor(obj));
2116
}
2217
}

SignalsDotnet/SignalsDotnet/Internals/Helpers/ObservableFromINotifyCollectionChanged.cs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,9 @@ internal static class ObservableFromINotifyCollectionChanged
1010
return new CollectionChangedObservable(collection);
1111
}
1212

13-
class CollectionChangedObservable : Observable<(object? sender, NotifyCollectionChangedEventArgs e)>
13+
sealed class CollectionChangedObservable(INotifyCollectionChanged notifyCollectionChanged) : Observable<(object? sender, NotifyCollectionChangedEventArgs e)>
1414
{
15-
readonly INotifyCollectionChanged _notifyCollectionChanged;
16-
17-
public CollectionChangedObservable(INotifyCollectionChanged notifyCollectionChanged)
18-
{
19-
_notifyCollectionChanged = notifyCollectionChanged;
20-
}
15+
readonly INotifyCollectionChanged _notifyCollectionChanged = notifyCollectionChanged;
2116

2217
protected override IDisposable SubscribeCore(Observer<(object? sender, NotifyCollectionChangedEventArgs e)> observer)
2318
{

SignalsDotnet/SignalsDotnet/Internals/Helpers/ObservableFromPropertyChanged.cs

Lines changed: 14 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -15,29 +15,21 @@ public static FromPropertyChangedObservableUnit OnPropertyChangedAsUnit<T>(this
1515
return new FromPropertyChangedObservableUnit(@this, futureChangesOnly);
1616
}
1717

18-
public class FromPropertyChangedObservableUnit : Observable<Unit>
18+
public class FromPropertyChangedObservableUnit(IReadOnlySignal signal, bool futureChangesOnly) : Observable<Unit>
1919
{
20-
readonly IReadOnlySignal _signal;
21-
readonly bool _futureChangesOnly;
22-
23-
public FromPropertyChangedObservableUnit(IReadOnlySignal signal, bool futureChangesOnly)
24-
{
25-
_signal = signal;
26-
_futureChangesOnly = futureChangesOnly;
27-
}
20+
readonly IReadOnlySignal _signal = signal;
21+
readonly bool _futureChangesOnly = futureChangesOnly;
2822

2923
protected override IDisposable SubscribeCore(Observer<Unit> observer)
3024
{
3125
return new FromPropertyChangedSubscriptionUnit(observer, this);
3226
}
3327

3428

35-
class FromPropertyChangedSubscriptionUnit : IDisposable
29+
sealed class FromPropertyChangedSubscriptionUnit : IDisposable
3630
{
3731
readonly Observer<Unit> _observer;
3832
readonly FromPropertyChangedObservableUnit _observable;
39-
readonly object _locker = new();
40-
bool _isDisposed;
4133

4234
public FromPropertyChangedSubscriptionUnit(Observer<Unit> observer, FromPropertyChangedObservableUnit observable)
4335
{
@@ -50,64 +42,30 @@ public FromPropertyChangedSubscriptionUnit(Observer<Unit> observer, FromProperty
5042
}
5143

5244
_observer.OnNext(Unit.Default);
53-
lock (_locker)
54-
{
55-
if (_isDisposed)
56-
{
57-
return;
58-
}
59-
60-
observable._signal.PropertyChanged += OnPropertyChanged;
61-
}
45+
observable._signal.PropertyChanged += OnPropertyChanged;
6246
}
6347

64-
void OnPropertyChanged(object? sender, PropertyChangedEventArgs e)
65-
{
66-
_observer.OnNext(Unit.Default);
67-
}
68-
69-
public void Dispose()
70-
{
71-
if (!_observable._futureChangesOnly)
72-
{
73-
lock (_locker)
74-
{
75-
if (_isDisposed)
76-
return;
77-
78-
_isDisposed = true;
79-
}
80-
}
81-
82-
_observable._signal.PropertyChanged -= OnPropertyChanged;
83-
}
48+
void OnPropertyChanged(object? sender, PropertyChangedEventArgs e) => _observer.OnNext(Unit.Default);
49+
public void Dispose() => _observable._signal.PropertyChanged -= OnPropertyChanged;
8450
}
8551
}
8652

8753

88-
public class FromPropertyChangedObservable<T> : Observable<T>
54+
public class FromPropertyChangedObservable<T>(IReadOnlySignal<T> signal, bool futureChangesOnly) : Observable<T>
8955
{
90-
readonly IReadOnlySignal<T> _signal;
91-
readonly bool _futureChangesOnly;
92-
93-
public FromPropertyChangedObservable(IReadOnlySignal<T> signal, bool futureChangesOnly)
94-
{
95-
_signal = signal;
96-
_futureChangesOnly = futureChangesOnly;
97-
}
56+
readonly IReadOnlySignal<T> _signal = signal;
57+
readonly bool _futureChangesOnly = futureChangesOnly;
9858

9959
protected override IDisposable SubscribeCore(Observer<T> observer)
10060
{
10161
return new FromPropertyChangedSubscription(observer, this);
10262
}
10363

10464

105-
class FromPropertyChangedSubscription : IDisposable
65+
sealed class FromPropertyChangedSubscription : IDisposable
10666
{
10767
readonly Observer<T> _observer;
10868
readonly FromPropertyChangedObservable<T> _observable;
109-
readonly object _locker = new();
110-
bool _isDisposed;
11169

11270
public FromPropertyChangedSubscription(Observer<T> observer, FromPropertyChangedObservable<T> observable)
11371
{
@@ -120,37 +78,12 @@ public FromPropertyChangedSubscription(Observer<T> observer, FromPropertyChanged
12078
}
12179

12280
_observer.OnNext(_observable._signal.Value);
123-
lock (_locker)
124-
{
125-
if (_isDisposed)
126-
{
127-
return;
128-
}
129-
130-
observable._signal.PropertyChanged += OnPropertyChanged;
131-
}
132-
}
133-
134-
void OnPropertyChanged(object? sender, PropertyChangedEventArgs e)
135-
{
136-
_observer.OnNext(_observable._signal.Value);
81+
observable._signal.PropertyChanged += OnPropertyChanged;
13782
}
13883

139-
public void Dispose()
140-
{
141-
if (!_observable._futureChangesOnly)
142-
{
143-
lock (_locker)
144-
{
145-
if (_isDisposed)
146-
return;
147-
148-
_isDisposed = true;
149-
}
150-
}
84+
void OnPropertyChanged(object? sender, PropertyChangedEventArgs e) => _observer.OnNext(_observable._signal.Value);
15185

152-
_observable._signal.PropertyChanged -= OnPropertyChanged;
153-
}
86+
public void Dispose() => _observable._signal.PropertyChanged -= OnPropertyChanged;
15487
}
15588
}
15689
}

0 commit comments

Comments
 (0)