Skip to content

more improvements #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TopSwagCode merged 1 commit into from
Feb 12, 2026
Merged

more improvements #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 70 additions & 36 deletions src/MinimalWorker.Generators/WorkerEmitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ public static string EmitSource(List<WorkerInvocationModel> workers)
sb.AppendLine(" // Worker state tracking for observable gauges");
sb.AppendLine(" private static readonly System.Collections.Concurrent.ConcurrentDictionary<string, WorkerState> _workerStates = new();");
sb.AppendLine();
sb.AppendLine(" // Cached snapshot for gauge enumeration optimization");
sb.AppendLine(" private static WorkerState[]? _cachedSnapshot;");
sb.AppendLine(" private static int _snapshotVersion;");
sb.AppendLine(" private static int _currentVersion;");
sb.AppendLine();
sb.AppendLine(" internal sealed class WorkerState");
sb.AppendLine(" {");
sb.AppendLine(" public string WorkerId { get; set; } = string.Empty;");
Expand All @@ -114,6 +119,24 @@ public static string EmitSource(List<WorkerInvocationModel> workers)
sb.AppendLine(" public long ConsecutiveFailures { get; set; }");
sb.AppendLine(" }");
sb.AppendLine();
sb.AppendLine(" /// <summary>");
sb.AppendLine(" /// Gets a cached snapshot of worker states, only re-allocating when workers are added/removed.");

Copilot AI Feb 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The emitted XML doc says the snapshot is only re-allocated when workers are added/removed, but the generated code invalidates the cache on DeactivateWorker (a state change, not an add/remove). Either update the comment to match the actual invalidation behavior, or avoid invalidating on deactivation if the goal is strictly “add/remove” (the snapshot contains references, so IsActive changes are already observable without reallocation).

Suggested change
sb.AppendLine(" /// Gets a cached snapshot of worker states, only re-allocating when workers are added/removed.");
sb.AppendLine(" /// Gets a cached snapshot of worker states. The snapshot is re-allocated whenever the worker state version changes (e.g., workers are added, removed, or their activation state changes).");

Copilot uses AI. Check for mistakes.
sb.AppendLine(" /// </summary>");
sb.AppendLine(" private static WorkerState[] GetWorkerStatesSnapshot()");
sb.AppendLine(" {");
sb.AppendLine(" var version = Volatile.Read(ref _currentVersion);");
sb.AppendLine(" var cached = _cachedSnapshot;");
sb.AppendLine(" if (cached != null && _snapshotVersion == version)");
sb.AppendLine(" {");
sb.AppendLine(" return cached;");
sb.AppendLine(" }");
sb.AppendLine(" // Version changed or no cache - create new snapshot");
sb.AppendLine(" var snapshot = _workerStates.Values.ToArray();");
sb.AppendLine(" _cachedSnapshot = snapshot;");
sb.AppendLine(" _snapshotVersion = version;");
Comment on lines +128 to +136

Copilot AI Feb 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated snapshot cache uses plain reads/writes of _cachedSnapshot and _snapshotVersion. Because these fields aren't accessed via Volatile.Read/Write (or another memory barrier), the JIT is allowed to reorder writes, which can lead to returning an out-of-date snapshot even when _currentVersion has advanced. Consider storing (version, snapshot) as a single immutable reference updated via Volatile.Write, or make both fields volatile and use Volatile.Read/Write consistently (or lock) to ensure correctness under concurrent metrics reads/worker registration.

Suggested change
sb.AppendLine(" var cached = _cachedSnapshot;");
sb.AppendLine(" if (cached != null && _snapshotVersion == version)");
sb.AppendLine(" {");
sb.AppendLine(" return cached;");
sb.AppendLine(" }");
sb.AppendLine(" // Version changed or no cache - create new snapshot");
sb.AppendLine(" var snapshot = _workerStates.Values.ToArray();");
sb.AppendLine(" _cachedSnapshot = snapshot;");
sb.AppendLine(" _snapshotVersion = version;");
sb.AppendLine(" var cached = Volatile.Read(ref _cachedSnapshot);");
sb.AppendLine(" var cachedVersion = Volatile.Read(ref _snapshotVersion);");
sb.AppendLine(" if (cached != null && cachedVersion == version)");
sb.AppendLine(" {");
sb.AppendLine(" return cached;");
sb.AppendLine(" }");
sb.AppendLine(" // Version changed or no cache - create new snapshot");
sb.AppendLine(" var snapshot = _workerStates.Values.ToArray();");
sb.AppendLine(" Volatile.Write(ref _cachedSnapshot, snapshot);");
sb.AppendLine(" Volatile.Write(ref _snapshotVersion, version);");

Copilot uses AI. Check for mistakes.
sb.AppendLine(" return snapshot;");
sb.AppendLine(" }");
sb.AppendLine();
sb.AppendLine(" internal static void RegisterWorker(string workerId, string workerName, string workerType)");
sb.AppendLine(" {");
sb.AppendLine(" _workerStates[workerId] = new WorkerState");
Expand All @@ -125,6 +148,8 @@ public static string EmitSource(List<WorkerInvocationModel> workers)
sb.AppendLine(" LastSuccessTimestamp = 0,");
sb.AppendLine(" ConsecutiveFailures = 0");
sb.AppendLine(" };");
sb.AppendLine(" // Invalidate cache when worker is registered");
sb.AppendLine(" Interlocked.Increment(ref _currentVersion);");
sb.AppendLine(" }");
sb.AppendLine();
sb.AppendLine(" internal static void RecordSuccess(string workerId)");
Expand All @@ -150,6 +175,8 @@ public static string EmitSource(List<WorkerInvocationModel> workers)
sb.AppendLine(" {");
sb.AppendLine(" state.IsActive = false;");
sb.AppendLine(" }");
sb.AppendLine(" // Invalidate cache when worker is deactivated");
sb.AppendLine(" Interlocked.Increment(ref _currentVersion);");
sb.AppendLine(" }");
sb.AppendLine();
sb.AppendLine(" /// <summary>");
Expand All @@ -164,8 +191,8 @@ public static string EmitSource(List<WorkerInvocationModel> workers)
sb.AppendLine();
sb.AppendLine(" private static System.Collections.Generic.IEnumerable<Measurement<int>> GetActiveWorkerMeasurements()");
sb.AppendLine(" {");
sb.AppendLine(" // Take a snapshot to avoid collection modified exception during enumeration");
sb.AppendLine(" foreach (var state in _workerStates.Values.ToArray())");
sb.AppendLine(" // Use cached snapshot to avoid allocation on every metrics export");
sb.AppendLine(" foreach (var state in GetWorkerStatesSnapshot())");
sb.AppendLine(" {");
sb.AppendLine(" var tags = new TagList");
sb.AppendLine(" {");
Expand All @@ -189,8 +216,8 @@ public static string EmitSource(List<WorkerInvocationModel> workers)
sb.AppendLine();
sb.AppendLine(" private static System.Collections.Generic.IEnumerable<Measurement<long>> GetLastSuccessMeasurements()");
sb.AppendLine(" {");
sb.AppendLine(" // Take a snapshot to avoid collection modified exception during enumeration");
sb.AppendLine(" foreach (var state in _workerStates.Values.ToArray())");
sb.AppendLine(" // Use cached snapshot to avoid allocation on every metrics export");
sb.AppendLine(" foreach (var state in GetWorkerStatesSnapshot())");
sb.AppendLine(" {");
sb.AppendLine(" if (state.LastSuccessTimestamp > 0)");
sb.AppendLine(" {");
Expand All @@ -216,8 +243,8 @@ public static string EmitSource(List<WorkerInvocationModel> workers)
sb.AppendLine();
sb.AppendLine(" private static System.Collections.Generic.IEnumerable<Measurement<long>> GetConsecutiveFailuresMeasurements()");
sb.AppendLine(" {");
sb.AppendLine(" // Take a snapshot to avoid collection modified exception during enumeration");
sb.AppendLine(" foreach (var state in _workerStates.Values.ToArray())");
sb.AppendLine(" // Use cached snapshot to avoid allocation on every metrics export");
sb.AppendLine(" foreach (var state in GetWorkerStatesSnapshot())");
sb.AppendLine(" {");
sb.AppendLine(" var tags = new TagList");
sb.AppendLine(" {");
Expand Down Expand Up @@ -285,11 +312,43 @@ private static void EmitWorkerLogMessages(StringBuilder sb)

private static void EmitWorkerExtension(StringBuilder sb, List<WorkerInvocationModel> workers)
{
// Build worker map first to determine signatures
int caseNum = 1;
var workerMap = new Dictionary<string, WorkerInvocationModel>();
var signatureToMethod = new Dictionary<string, int>();

foreach (var worker in workers)
{
// Build signature from worker parameters - strip global:: prefix and normalize spacing to match runtime format
// Runtime uses FormatTypeName which joins generic args with "," (no space), so we must do the same
var paramTypes = string.Join(",", worker.Parameters.Select(p => p.Type.Replace("global::", "").Replace(", ", ",")));
var signature = $"{worker.Type}:{paramTypes}";
Comment on lines +322 to +325

Copilot AI Feb 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signature normalization here may not match the runtime BackgroundWorkerExtensions.FormatTypeName output for nested types: Type.FullName uses + between declaring and nested types, while Roslyn’s FullyQualifiedFormat typically uses .. That would produce a registration.Signature that never matches any generated initializer, and the worker won’t start. Consider normalizing nested type separators consistently on both sides (e.g., replace + with . in FormatTypeName, or adjust generator signature formatting to match Type.FullName).

Copilot uses AI. Check for mistakes.

if (!workerMap.ContainsKey(signature))
{
workerMap[signature] = worker;
signatureToMethod[signature] = caseNum;
caseNum++;
}
}

sb.AppendLine("/// <summary>");
sb.AppendLine("/// Generated class to wire up background workers using a module initializer.");
sb.AppendLine("/// </summary>");
sb.AppendLine("internal static class GeneratedBackgroundWorkerInitializer");
sb.AppendLine("{");

// Generate static dictionary for O(1) signature dispatch
sb.AppendLine(" // O(1) signature dispatch dictionary - initialized once, used for all lookups");
sb.AppendLine(" private static readonly System.Collections.Generic.Dictionary<string, Action<BackgroundWorkerExtensions.WorkerRegistration>> _workerInitializers = new()");
sb.AppendLine(" {");
foreach (var kvp in signatureToMethod)
{
sb.AppendLine($" {{ \"{kvp.Key}\", InitializeWorker_{kvp.Value} }},");
}
sb.AppendLine(" };");
sb.AppendLine();

sb.AppendLine(" [System.Runtime.CompilerServices.ModuleInitializer]");
sb.AppendLine(" internal static void Initialize()");
sb.AppendLine(" {");
Expand All @@ -303,50 +362,25 @@ private static void EmitWorkerExtension(StringBuilder sb, List<WorkerInvocationM
sb.AppendLine(" .Where(r => r.Host == host)");
sb.AppendLine(" .ToList();");
sb.AppendLine();
sb.AppendLine(" // Initialize each worker based on its parameter signature");
sb.AppendLine(" // Initialize each worker using O(1) dictionary lookup");
sb.AppendLine(" foreach (var registration in registrations)");
sb.AppendLine(" {");

// Generate a switch statement to match workers by their unique signature
sb.AppendLine(" switch (registration.Signature)");
sb.AppendLine(" if (_workerInitializers.TryGetValue(registration.Signature, out var initializer))");
sb.AppendLine(" {");

// Create a case for each unique worker signature
int caseNum = 1;
var workerMap = new Dictionary<string, WorkerInvocationModel>();
foreach (var worker in workers)
{
// Build signature from worker parameters - strip global:: prefix and normalize spacing to match runtime format
// Runtime uses FormatTypeName which joins generic args with "," (no space), so we must do the same
var paramTypes = string.Join(",", worker.Parameters.Select(p => p.Type.Replace("global::", "").Replace(", ", ",")));
var signature = $"{worker.Type}:{paramTypes}";

if (!workerMap.ContainsKey(signature))
{
workerMap[signature] = worker;
sb.AppendLine($" case \"{signature}\":");
sb.AppendLine($" InitializeWorker_{caseNum}(registration);");
sb.AppendLine(" break;");
caseNum++;
}
}

sb.AppendLine(" default:");
sb.AppendLine(" // No matching worker initializer - this shouldn't happen");
sb.AppendLine(" break;");
sb.AppendLine(" initializer(registration);");
sb.AppendLine(" }");
sb.AppendLine(" }");
sb.AppendLine(" }");
sb.AppendLine();

// Emit worker initialization methods - one for each unique signature
caseNum = 1;
foreach (var kvp in workerMap)
{
EmitWorkerInitializer(sb, kvp.Value, caseNum);
Comment on lines 376 to 380

Copilot AI Feb 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

workerMap is a Dictionary, and later you generate initializer method numbers by enumerating workerMap while the signature→method mapping is produced from a different dictionary (signatureToMethod). Since Dictionary enumeration order is not guaranteed by contract, it’s possible for the emitted InitializeWorker_{n} methods to not align with the numbers referenced in _workerInitializers, resulting in the wrong initializer being invoked for a signature (and likely invalid casts at runtime). Use a deterministic ordered sequence (e.g., a List of signatures in insertion order) to drive both the _workerInitializers entries and the emitted InitializeWorker_{n} methods.

Copilot uses AI. Check for mistakes.
caseNum++;
}

sb.AppendLine("}");
}

Expand Down
Loading
Loading