From 525408d4a92c1381aac19d386492fbb697fc95a9 Mon Sep 17 00:00:00 2001 From: Erik Zettersten Date: Wed, 11 Oct 2023 10:50:05 -0400 Subject: [PATCH] adds System.Threading.Timer for aggregate change delay --- ReadMe.md | 6 ++++ .../OutOfProcess/OutOfProcessNodeJSService.cs | 31 +++++++++++++++++++ .../OutOfProcessNodeJSServiceOptions.cs | 8 +++++ src/NodeJS/packages.lock.json | 14 +++++++++ test/NodeJS/packages.lock.json | 14 +++++++++ 5 files changed, 73 insertions(+) diff --git a/ReadMe.md b/ReadMe.md index e614108..2f3198c 100644 --- a/ReadMe.md +++ b/ReadMe.md @@ -1471,6 +1471,12 @@ Also, invocations complete using the outdated version of your script. Weigh thes This value does nothing if `OutOfProcessNodeJSServiceOptions.EnableFileWatching` is `false` and `OutOfProcessNodeJSServiceOptions.NumProcessRetries` is 0. Defaults to `true`. +##### OutOfProcessNodeJSServiceOptions.AggregateTimeout + +Add a delay before restarting the NodeJS service once the first file changed. This allows OutOfProcessNodeJSService to aggregate any other changes made during this time period into one rebuild. +```csharp +public int AggregateTimeout { get; set; } +``` diff --git a/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/OutOfProcessNodeJSService.cs b/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/OutOfProcessNodeJSService.cs index c52dddc..d4a65f7 100644 --- a/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/OutOfProcessNodeJSService.cs +++ b/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/OutOfProcessNodeJSService.cs @@ -45,9 +45,12 @@ public abstract class OutOfProcessNodeJSService : INodeJSService private readonly int _invocationTimeoutMS; private readonly ConcurrentDictionary _trackedInvokeTasks; // TODO use ConcurrentSet when it's available - https://github.com/dotnet/runtime/issues/16443 private readonly bool _trackInvokeTasks; + private readonly object _fileChangeAggregateLock = new(); + private readonly int _fileChangeAggregateDelayInMilliseconds; private bool _disposed; private volatile INodeJSProcess? _nodeJSProcess; // Volatile since it's used in a double checked lock + private Timer? _fileChangeAggregateTimer; /// /// This regex is used to determine successful initialization of the process. @@ -88,6 +91,8 @@ protected OutOfProcessNodeJSService(INodeJSProcessFactory nodeProcessFactory, _serverScriptAssembly = serverScriptAssembly; Logger = logger; + _fileChangeAggregateDelayInMilliseconds = _options.AggregateTimeout; + _debugLoggingEnabled = Logger.IsEnabled(LogLevel.Debug); _warningLoggingEnabled = Logger.IsEnabled(LogLevel.Warning); _infoLoggingEnabled = Logger.IsEnabled(LogLevel.Information); @@ -576,12 +581,38 @@ internal virtual void FileChangedHandler() Logger.LogDebug(string.Format(Strings.LogDebug_FileChangedHandlerInvokedForProcess, _nodeJSProcess?.SafeID)); } + lock (_fileChangeAggregateLock) + { + // If the timer is already running, disable it. + _fileChangeAggregateTimer?.Change(Timeout.Infinite, Timeout.Infinite); + + // Start or restart the timer. + if (_fileChangeAggregateTimer == null) + { + _fileChangeAggregateTimer = new Timer(ExecuteFileChangeAggregate, null, _fileChangeAggregateDelayInMilliseconds, Timeout.Infinite); + } + else + { + _fileChangeAggregateTimer.Change(_fileChangeAggregateDelayInMilliseconds, Timeout.Infinite); + } + } + } + + // Delegate for timer execution + private void ExecuteFileChangeAggregate(object? state) + { #pragma warning disable CS4014 // No need to await // // Note that we need to reconnect even if we've just connected so that the changed file is loaded. MoveToNewProcessAsync(true); #pragma warning restore CS4014 + + lock (_fileChangeAggregateLock) + { + _fileChangeAggregateTimer?.Dispose(); + _fileChangeAggregateTimer = null; + } } // Just connected refers to the situation where _nodeJSProcess.Connected is true but _connectingLock has not been released diff --git a/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/OutOfProcessNodeJSServiceOptions.cs b/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/OutOfProcessNodeJSServiceOptions.cs index e6753dd..79e2590 100644 --- a/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/OutOfProcessNodeJSServiceOptions.cs +++ b/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/OutOfProcessNodeJSServiceOptions.cs @@ -129,5 +129,13 @@ public class OutOfProcessNodeJSServiceOptions /// Defaults to true. /// public bool GracefulProcessShutdown { get; set; } = true; + + /// + /// Add a delay before restarting the NodeJS service once the first file changed. This allows OutOfProcessNodeJSService to aggregate any other changes made during this time period into one rebuild. + /// + /// Value is set in milliseconds + /// This value does nothing if is false + /// Defaults to 600. + public int AggregateTimeout { get; set; } = 600; } } diff --git a/src/NodeJS/packages.lock.json b/src/NodeJS/packages.lock.json index abbfc5b..3ead6a0 100644 --- a/src/NodeJS/packages.lock.json +++ b/src/NodeJS/packages.lock.json @@ -196,6 +196,15 @@ "Microsoft.Extensions.Options": "6.0.0" } }, + "Microsoft.NETFramework.ReferenceAssemblies": { + "type": "Direct", + "requested": "[1.0.3, )", + "resolved": "1.0.3", + "contentHash": "vUc9Npcs14QsyOD01tnv/m8sQUnGTGOw1BCmKcv77LBJY7OxhJ+zJF7UD/sCL3lYNFuqmQEVlkfS4Quif6FyYg==", + "dependencies": { + "Microsoft.NETFramework.ReferenceAssemblies.net461": "1.0.3" + } + }, "Microsoft.SourceLink.GitHub": { "type": "Direct", "requested": "[1.1.1, )", @@ -335,6 +344,11 @@ "System.Runtime.CompilerServices.Unsafe": "6.0.0" } }, + "Microsoft.NETFramework.ReferenceAssemblies.net461": { + "type": "Transitive", + "resolved": "1.0.3", + "contentHash": "AmOJZwCqnOCNp6PPcf9joyogScWLtwy0M1WkqfEQ0M9nYwyDD7EX9ZjscKS5iYnyvteX7kzSKFCKt9I9dXA6mA==" + }, "Microsoft.SourceLink.Common": { "type": "Transitive", "resolved": "1.1.1", diff --git a/test/NodeJS/packages.lock.json b/test/NodeJS/packages.lock.json index ac1d21e..addce9c 100644 --- a/test/NodeJS/packages.lock.json +++ b/test/NodeJS/packages.lock.json @@ -1376,6 +1376,15 @@ "Microsoft.CodeCoverage": "17.0.0" } }, + "Microsoft.NETFramework.ReferenceAssemblies": { + "type": "Direct", + "requested": "[1.0.3, )", + "resolved": "1.0.3", + "contentHash": "vUc9Npcs14QsyOD01tnv/m8sQUnGTGOw1BCmKcv77LBJY7OxhJ+zJF7UD/sCL3lYNFuqmQEVlkfS4Quif6FyYg==", + "dependencies": { + "Microsoft.NETFramework.ReferenceAssemblies.net461": "1.0.3" + } + }, "Moq": { "type": "Direct", "requested": "[4.16.1, )", @@ -1527,6 +1536,11 @@ "System.Runtime.CompilerServices.Unsafe": "6.0.0" } }, + "Microsoft.NETFramework.ReferenceAssemblies.net461": { + "type": "Transitive", + "resolved": "1.0.3", + "contentHash": "AmOJZwCqnOCNp6PPcf9joyogScWLtwy0M1WkqfEQ0M9nYwyDD7EX9ZjscKS5iYnyvteX7kzSKFCKt9I9dXA6mA==" + }, "System.Buffers": { "type": "Transitive", "resolved": "4.5.1",