Skip to content

Commit

Permalink
fix(mvux): Properties update might be defferred too long if dispatche…
Browse files Browse the repository at this point in the history
…r is under heavy stress
  • Loading branch information
dr1rrb committed Apr 12, 2024
1 parent 454b9d4 commit 042f44f
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 24 deletions.
5 changes: 4 additions & 1 deletion src/Uno.Extensions.Core/Option.T.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ public override string ToString()
{
OptionType.Undefined => $"Undefined<{typeof(T).Name}>",
OptionType.None => $"None<{typeof(T).Name}>",
_ => $"Some({_value switch { string => _value, IEnumerable enumerable => string.Join(",", enumerable.Cast<object>()), _ => _value }})",
_ when _value is null => "Some(--null--)",
_ when _value is string str => $"Some({str})",
_ when _value is IEnumerable enumerable => $"Some({string.Join(",", enumerable.Cast<object>())})",
_ => $"Some({_value})",
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public Dispatcher(DispatcherQueue queue)

/// <inheritdoc />
public bool TryEnqueue(Action action)
=> _queue.TryEnqueue(() => action());
=> _queue.TryEnqueue(Unsafe.As<DispatcherQueueHandler>(action));

/// <inheritdoc />
public async ValueTask<TResult> ExecuteAsync<TResult>(AsyncFunc<TResult> action, CancellationToken ct)
Expand Down
5 changes: 3 additions & 2 deletions src/Uno.Extensions.Reactive/Core/Internal/FeedSubscription.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Uno.Extensions.Reactive.Logging;
using Uno.Extensions.Reactive.Operators;
using Uno.Extensions.Reactive.Utils;
using Uno.Extensions.Reactive.Utils.Logging;

namespace Uno.Extensions.Reactive.Core;

Expand All @@ -35,7 +36,7 @@ public FeedSubscription(ISignal<Message<T>> feed, SourceContext rootContext)
isInitialSyncValuesSkippingAllowed: true);
}

string ISourceContextOwner.Name => $"Sub on '{_feed}' for ctx '{_context.Parent!.Owner.Name}'.";
string ISourceContextOwner.Name => $"Sub on '{LogHelper.GetIdentifier(_feed)}' for ctx '{_context.Parent!.Owner.Name}'.";

IDispatcher? ISourceContextOwner.Dispatcher => null;

Expand Down Expand Up @@ -64,7 +65,7 @@ public async IAsyncEnumerable<Message<T>> GetMessages(SourceContext subscriberCo
{
// We make sure that even if we are replaying a previous message, the changes collection contains all keys.
isFirstMessage = false;
yield return Message<T>.Initial.OverrideBy(msg);
yield return Message<T>.Initial.OverrideBy(msg);
}
else
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -12,6 +14,7 @@
using Uno.Extensions.Reactive.Events;
using Uno.Extensions.Reactive.Logging;
using Uno.Extensions.Reactive.Utils;
using Uno.Extensions.Reactive.Utils.Logging;

namespace Uno.Extensions.Reactive.Bindings;

Expand Down Expand Up @@ -127,36 +130,48 @@ async void ViewModelToView(Action<TProperty> updated)
{
try
{
var initialValue = stateImpl.Current.Current.Data.SomeOrDefault(GetDefaultValueForBindings<TProperty>());
var defaultValue = GetDefaultValueForBindings<TProperty>();
var value = stateImpl.Current.Current.Data.SomeOrDefault(defaultValue);

// We run the update sync in setup, no matter the thread
updated(initialValue);
updated(value);

var ct = stateImpl.Context.Token;
var source = FeedUIHelper.GetSource(stateImpl, stateImpl.Context);
var dispatcher = await _dispatcher.GetFirstResolved(ct).ConfigureAwait(false);

dispatcher.TryEnqueue(async () =>
var updateScheduled = 0;

// Note: We use for each here to deduplicate updates in case of fast updates of the source.
// This also ensure to not wait to for the UI thread before fetching MoveNext the source.
_ = source
.ForEachAsync(OnMessage, ct)
.ContinueWith(
enumeration =>
{
this.Log().Error(
enumeration.Exception!,
$"Synchronization from ViewModel to View of '{propertyName}' failed."
+ "(This is a final error, changes made in the VM are no longer propagated to the View.)");
},
TaskContinuationOptions.OnlyOnFaulted);
void OnMessage(Message<TProperty> msg)
{
try
if (msg.Changes.Contains(MessageAxis.Data) && !(msg.Current.Get(BindingSource)?.Equals((this, propertyName)) ?? false))
{
// Note: No needs to use .WithCancellation() here as we are enumerating the stateImp which is going to be disposed anyway.
await foreach (var msg in source.WithCancellation(ct).ConfigureAwait(true))
value = msg.Current.Data.SomeOrDefault(defaultValue);
if (Interlocked.CompareExchange(ref updateScheduled, 1, 0) is 0)
{
if (msg.Changes.Contains(MessageAxis.Data) && !(msg.Current.Get(BindingSource)?.Equals((this, propertyName)) ?? false))
{
updated(msg.Current.Data.SomeOrDefault(GetDefaultValueForBindings<TProperty>()));
_propertyChanged.Raise(new PropertyChangedEventArgs(propertyName));
}
dispatcher.TryEnqueue(UpdateValue);
}
}
catch (Exception error)
{
this.Log().Error(
error,
$"Synchronization from ViewModel to View of '{propertyName}' failed."
+ "(This is a final error, changes made in the VM are no longer propagated to the View.)");
}
});
}

void UpdateValue()
{
updateScheduled = 0;
updated(value);
_propertyChanged.Raise(new PropertyChangedEventArgs(propertyName));
}
}
catch (Exception error)
{
Expand Down
29 changes: 29 additions & 0 deletions src/Uno.Extensions.Reactive/Utils/Logging/LogHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
using System;
using System.Linq;
using System.Threading;
using Uno.Extensions.Reactive;

namespace Uno.Extensions.Reactive.Utils.Logging;

internal static class LogHelper
{
public static string GetIdentifier(object? obj)
=> obj switch
{
null => "--null--",
#if DEBUG
_ => $"{GetTypeName(obj)}-{obj.GetHashCode():X8}",
#else
_ => obj.ToString()
#endif
};

public static string GetTypeName(object obj)
=> obj.GetType() switch
{
{ IsGenericType: true } type => $"{type.Name}<{string.Join(", ", type.GenericTypeArguments.Select(GetTypeName))}>",
{ IsArray: true } type => $"{GetTypeName(type.GetElementType()!)}[]",
{ IsValueType: true } type => type.ToString(),
{ } type => type.Name
};
}

0 comments on commit 042f44f

Please sign in to comment.