Skip to content

Commit

Permalink
Merge pull request #19005 from unoplatform/dev/dr/hrRetry
Browse files Browse the repository at this point in the history
fix(hr): Improve retry logic when using UpdateFile in case of NoChanges
  • Loading branch information
dr1rrb authored Dec 11, 2024
2 parents 13630a7 + f8d848b commit d1aeac1
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ void LoadInfos(HotReloadServerOperation? operation)
/// </summary>
private class HotReloadServerOperation
{
public readonly static int DefaultAutoRetryIfNoChangesAttempts = 3;

public readonly static TimeSpan DefaultAutoRetryIfNoChangesDelay = TimeSpan.FromMilliseconds(500);

// Delay to wait without any update to consider operation was aborted.
private static readonly TimeSpan _timeoutDelay = TimeSpan.FromSeconds(30);

Expand All @@ -219,7 +223,11 @@ private class HotReloadServerOperation
private ImmutableHashSet<string> _filePaths;
private int /* HotReloadResult */ _result = -1;
private CancellationTokenSource? _deferredCompletion;

// In VS we forcefully request to VS to hot-reload application, but in some cases the changes are not detected by VS and it returns a NoChanges result.
// In such cases we can retry the hot-reload request to VS to let it process the file updates.
private int _noChangesRetry;
private TimeSpan _noChangesRetryDelay = DefaultAutoRetryIfNoChangesDelay;

public long Id { get; } = Interlocked.Increment(ref _count);

Expand Down Expand Up @@ -294,8 +302,11 @@ public bool TryMerge(ImmutableHashSet<string> filePaths)
/// <summary>
/// Configure a simple auto-retry strategy if no changes are detected.
/// </summary>
public void EnableAutoRetryIfNoChanges()
=> _noChangesRetry = 1;
public void EnableAutoRetryIfNoChanges(int? attempts, TimeSpan? delay)
{
_noChangesRetry = attempts ?? DefaultAutoRetryIfNoChangesAttempts;
_noChangesRetryDelay = delay ?? DefaultAutoRetryIfNoChangesDelay;
}

/// <summary>
/// As errors might get a bit after the complete from the IDE, we can defer the completion of the operation.
Expand All @@ -322,10 +333,17 @@ private async ValueTask Complete(HotReloadServerResult result, Exception? except
{
if (_result is -1
&& result is HotReloadServerResult.NoChanges
&& Interlocked.Decrement(ref _noChangesRetry) >= 0
&& await _owner.RequestHotReloadToIde(Id))
&& Interlocked.Decrement(ref _noChangesRetry) >= 0)
{
return;
if (_noChangesRetryDelay is { TotalMilliseconds: > 0 })
{
await Task.Delay(_noChangesRetryDelay);
}

if (await _owner.RequestHotReloadToIde(Id))
{
return;
}
}

Debug.Assert(result != HotReloadServerResult.InternalError || exception is not null); // For internal error we should always provide an exception!
Expand Down Expand Up @@ -463,7 +481,7 @@ private async Task ProcessUpdateFile(UpdateFile message)
};
if ((int)result < 300 && !message.IsForceHotReloadDisabled)
{
hotReload.EnableAutoRetryIfNoChanges();
hotReload.EnableAutoRetryIfNoChanges(message.ForceHotReloadAttempts, message.ForceHotReloadDelay);
await RequestHotReloadToIde(hotReload.Id);
}

Expand Down Expand Up @@ -579,7 +597,11 @@ async Task WaitForFileUpdated()
{
if (e.FullPath.Equals(file.FullName, StringComparison.OrdinalIgnoreCase))
{
await Task.Delay(500);
if ((message.ForceHotReloadDelay ?? HotReloadServerOperation.DefaultAutoRetryIfNoChangesDelay) is { TotalMilliseconds: > 0 } delay)
{
await Task.Delay(delay);
}

tcs.TrySetResult();
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ public record struct UpdateRequest(
/// <remarks>This includes the time to apply the delta locally and then to run all local handlers.</remarks>
public TimeSpan LocalHotReloadTimeout { get; set; } = TimeSpan.FromSeconds(5);

/// <summary>
/// When <see cref="WaitForHotReload"/> the delay to wait before retrying a hot-reload in Visual Studio if no changes are detected.
/// </summary>
public TimeSpan? HotReloadNoChangesRetryDelay { get; set; }

/// <summary>
/// When <see cref="WaitForHotReload"/> the number of times to retry the hot reload in Visual Studio if no changes are detected.
/// </summary>
public int? HotReloadNoChangesRetryAttempts { get; set; }

public UpdateRequest WithExtendedTimeouts(float? factor = null)
{
factor ??= Debugger.IsAttached ? 10 : 30;
Expand Down Expand Up @@ -116,7 +126,14 @@ public async Task<UpdateResult> TryUpdateFileAsync(UpdateRequest req, Cancellati
// As the local HR is not really ID trackable (trigger by VS without any ID), we capture the current ID here to make sure that if HR completes locally before we get info from the server, we won't miss it.
var currentLocalHrId = GetCurrentLocalHotReloadId();

var request = new UpdateFile { FilePath = req.FilePath, OldText = req.OldText, NewText = req.NewText };
var request = new UpdateFile
{
FilePath = req.FilePath,
OldText = req.OldText,
NewText = req.NewText,
ForceHotReloadDelay = req.HotReloadNoChangesRetryDelay,
ForceHotReloadAttempts = req.HotReloadNoChangesRetryAttempts
};
var response = await UpdateFileCoreAsync(request, req.ServerUpdateTimeout, ct);

if (response.Result is FileUpdateResult.NoChanges)
Expand Down
12 changes: 12 additions & 0 deletions src/Uno.UI.RemoteControl/HotReload/Messages/UpdateFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@ public class UpdateFile : IMessage
[JsonProperty]
public bool IsForceHotReloadDisabled { get; set; }

/// <summary>
/// The delay to wait before forcing (**OR RETRYING**) a hot reload in Visual Studio.
/// </summary>
[JsonProperty]
public TimeSpan? ForceHotReloadDelay { get; set; }

/// <summary>
/// Number of times to retry the hot reload in Visual Studio **if not changes are detected**.
/// </summary>
[JsonProperty]
public int? ForceHotReloadAttempts { get; set; }

[JsonIgnore]
public string Scope => WellKnownScopes.HotReload;

Expand Down

0 comments on commit d1aeac1

Please sign in to comment.