Reduce ServerMode notification allocations - #10670

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications
Aug 24, 2026
Merged

Reduce ServerMode notification allocations#10670
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • replace the TestNodeStateChangedEventArgs array LINQ projection with a capacity-sized List<object> fill while preserving null/empty behavior and runtime collection typing
  • isolate the aggregator's captured filtering predicate so batches without terminal states avoid its closure allocation
  • preserve the existing filtered LINQ materialization because pre-sized array/list alternatives increased allocation for large, heavily suppressed batches
  • add focused coverage for exact JSON shape/order, object typing, null and empty changes, retained identity/order, predicate behavior, and source immutability

Performance

A standalone faithful-shape harness measured median time and allocation over null, empty, 1, 8, 128, and 1,024 item inputs on .NET 8 and .NET Framework 4.6.2.

  • serializer, 8 items: 2,336 B -> 2,232 B on .NET 8; approximately 2,367 B -> 2,247 B on .NET Framework
  • serializer, 1,024 items: 286,984 B -> 278,584 B on .NET 8; approximately 287,857 B -> 279,413 B on .NET Framework
  • aggregator without terminal states: 24 B less allocation per batch on .NET 8, with the terminal/filtering path retaining its prior allocation shape

The serializer percentages are intentionally omitted because the harness models the container work faithfully but uses a smaller per-item payload than production. .NET Framework allocation values are approximate because its counter is AppDomain-wide.

Validation

.\build.cmd -projects .\test\UnitTests\Microsoft.Testing.Platform.UnitTests\Microsoft.Testing.Platform.UnitTests.csproj -test -bl

Passed on net8.0, net9.0, and net462 with 0 warnings and 0 errors.

Closes#10669

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 22, 2026 18:19
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 22, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Reduces ServerMode notification allocations while preserving serialization and aggregation behavior.

Changes:

  • Replaces serializer LINQ projection with a capacity-sized list.
  • Isolates the aggregator’s captured filtering predicate.
  • Adds serialization and aggregation regression tests.
Show a summary per file
FileDescription
SerializerUtilities.TestNodeSerializers.csOptimizes change serialization.
TestNodeStateChangeAggregator.csAvoids closure allocation for non-terminal batches.
FormatterUtilitiesTests.csTests serialization shape, typing, order, and null handling.
TestNodeStateChangeAggregatorTests.csTests repeated aggregation behavior.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1f616dbd-edac-4ae9-b58a-b0d7b4739297
CopilotAI review requested due to automatic review settings August 22, 2026 21:58
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10670

GradeTestMutationNotesHow to improve
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
DoesNotMutateBufferedChanges
4/4 killedMutates a buffered terminal item between two builds and asserts the previously suppressed in-progress item reappears in the right slot, proving the buffer itself is never mutated.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
Changes_
PreservesOrderObjectTypesAndSource
3/3 killedVerifies element order, UID values, and reference identity of the original source array against both the intermediate dictionary and the final JSON string, directly exercising the new indexed serialization loop.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
EmptyChanges_
PreservesEmptyObjectList
2/2 killedConfirms an empty (non-null) changes array serializes to [] rather than null, guarding the new null-check branch.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
NullChanges_
PreservesNull
2/2 killedConfirms a null changes array is preserved as null rather than becoming an empty list, guarding the new null-check branch.

Summary: This PR refactors TestNodeStateChangeAggregator.BuildAggregatedChange and the
TestNodeStateChangedEventArgs serializer to use an indexed loop instead of LINQ. All four new/
modified tests are well-targeted regression tests for the exact behaviors the refactor could
break (null vs. empty changes, element order/identity preservation, and buffer non-mutation
across repeated aggregation calls). No high-confidence actionable findings — all four tests
graded A with no inline suggestions.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 73.3 AIC · ⌖ 3.44 AIC · ⊞ 16.9K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10670

Parallelization — one row per test assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Platform.UnitTestsMethodLevel (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs)CPU countcoverable once the parallel-safety analyzers ship (attribute-based opt-in)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

Nothing to flag. Both changed test files add pure, instance-scoped test methods with no cross-test coordination concerns:

  • TestNodeStateChangeAggregatorTests.BuildAggregatedChange_DoesNotMutateBufferedChanges (new) creates its own local TestNodeStateChangeAggregator instance and only mutates a locally-owned TestNode.Properties._testNodeStateProperty on a TestNodeUpdateMessage it constructed itself in the test body — no shared/static state, no environment variables, no filesystem paths, no Console/culture mutation.
  • FormatterUtilitiesTests (new Serialize_TestNodeStateChangedEventArgs_* tests) exercise SerializerUtilities.Serialize against locally-constructed messages via the instance-level _formatter field created once per test-class instantiation (MSTest constructs the class per test). SerializerUtilities's backing Dictionary<Type, IObjectSerializer> fields are populated once in a static constructor and only read afterward by every test in the assembly — no test mutates them, so there is no cross-test write/write or read/write race even under MethodLevel.
  • No [ResourceLock], [DoNotParallelize], or assembly-level parallelization declaration was added, removed, or otherwise touched by this PR.

No production-code changes in this PR (SerializerUtilities.TestNodeSerializers.cs, TestNodeStateChangeAggregator.cs) introduce process-global mutation either — both are pure refactors of allocation patterns (capacity-sized list construction, extracted LINQ filter method) with no new shared state.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 51.7 AIC · ⌖ 9.39 AIC · ⊞ 24.8K · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@Evangelink
Amaury Levé (Evangelink) merged commit 39bf3c0 into mainAug 24, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-servermode-notifications branch August 24, 2026 08:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[perf-improver] Avoid LINQ allocations on ServerMode per-notification hot path

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Reduce ServerMode notification allocations - #10670

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications
Aug 24, 2026
Merged

Reduce ServerMode notification allocations#10670
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • replace the TestNodeStateChangedEventArgs array LINQ projection with a capacity-sized List<object> fill while preserving null/empty behavior and runtime collection typing
  • isolate the aggregator's captured filtering predicate so batches without terminal states avoid its closure allocation
  • preserve the existing filtered LINQ materialization because pre-sized array/list alternatives increased allocation for large, heavily suppressed batches
  • add focused coverage for exact JSON shape/order, object typing, null and empty changes, retained identity/order, predicate behavior, and source immutability

Performance

A standalone faithful-shape harness measured median time and allocation over null, empty, 1, 8, 128, and 1,024 item inputs on .NET 8 and .NET Framework 4.6.2.

  • serializer, 8 items: 2,336 B -> 2,232 B on .NET 8; approximately 2,367 B -> 2,247 B on .NET Framework
  • serializer, 1,024 items: 286,984 B -> 278,584 B on .NET 8; approximately 287,857 B -> 279,413 B on .NET Framework
  • aggregator without terminal states: 24 B less allocation per batch on .NET 8, with the terminal/filtering path retaining its prior allocation shape

The serializer percentages are intentionally omitted because the harness models the container work faithfully but uses a smaller per-item payload than production. .NET Framework allocation values are approximate because its counter is AppDomain-wide.

Validation

.\build.cmd -projects .\test\UnitTests\Microsoft.Testing.Platform.UnitTests\Microsoft.Testing.Platform.UnitTests.csproj -test -bl

Passed on net8.0, net9.0, and net462 with 0 warnings and 0 errors.

Closes#10669

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 22, 2026 18:19
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 22, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Reduces ServerMode notification allocations while preserving serialization and aggregation behavior.

Changes:

  • Replaces serializer LINQ projection with a capacity-sized list.
  • Isolates the aggregator’s captured filtering predicate.
  • Adds serialization and aggregation regression tests.
Show a summary per file
FileDescription
SerializerUtilities.TestNodeSerializers.csOptimizes change serialization.
TestNodeStateChangeAggregator.csAvoids closure allocation for non-terminal batches.
FormatterUtilitiesTests.csTests serialization shape, typing, order, and null handling.
TestNodeStateChangeAggregatorTests.csTests repeated aggregation behavior.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1f616dbd-edac-4ae9-b58a-b0d7b4739297
CopilotAI review requested due to automatic review settings August 22, 2026 21:58
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10670

GradeTestMutationNotesHow to improve
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
DoesNotMutateBufferedChanges
4/4 killedMutates a buffered terminal item between two builds and asserts the previously suppressed in-progress item reappears in the right slot, proving the buffer itself is never mutated.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
Changes_
PreservesOrderObjectTypesAndSource
3/3 killedVerifies element order, UID values, and reference identity of the original source array against both the intermediate dictionary and the final JSON string, directly exercising the new indexed serialization loop.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
EmptyChanges_
PreservesEmptyObjectList
2/2 killedConfirms an empty (non-null) changes array serializes to [] rather than null, guarding the new null-check branch.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
NullChanges_
PreservesNull
2/2 killedConfirms a null changes array is preserved as null rather than becoming an empty list, guarding the new null-check branch.

Summary: This PR refactors TestNodeStateChangeAggregator.BuildAggregatedChange and the
TestNodeStateChangedEventArgs serializer to use an indexed loop instead of LINQ. All four new/
modified tests are well-targeted regression tests for the exact behaviors the refactor could
break (null vs. empty changes, element order/identity preservation, and buffer non-mutation
across repeated aggregation calls). No high-confidence actionable findings — all four tests
graded A with no inline suggestions.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 73.3 AIC · ⌖ 3.44 AIC · ⊞ 16.9K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10670

Parallelization — one row per test assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Platform.UnitTestsMethodLevel (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs)CPU countcoverable once the parallel-safety analyzers ship (attribute-based opt-in)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

Nothing to flag. Both changed test files add pure, instance-scoped test methods with no cross-test coordination concerns:

  • TestNodeStateChangeAggregatorTests.BuildAggregatedChange_DoesNotMutateBufferedChanges (new) creates its own local TestNodeStateChangeAggregator instance and only mutates a locally-owned TestNode.Properties._testNodeStateProperty on a TestNodeUpdateMessage it constructed itself in the test body — no shared/static state, no environment variables, no filesystem paths, no Console/culture mutation.
  • FormatterUtilitiesTests (new Serialize_TestNodeStateChangedEventArgs_* tests) exercise SerializerUtilities.Serialize against locally-constructed messages via the instance-level _formatter field created once per test-class instantiation (MSTest constructs the class per test). SerializerUtilities's backing Dictionary<Type, IObjectSerializer> fields are populated once in a static constructor and only read afterward by every test in the assembly — no test mutates them, so there is no cross-test write/write or read/write race even under MethodLevel.
  • No [ResourceLock], [DoNotParallelize], or assembly-level parallelization declaration was added, removed, or otherwise touched by this PR.

No production-code changes in this PR (SerializerUtilities.TestNodeSerializers.cs, TestNodeStateChangeAggregator.cs) introduce process-global mutation either — both are pure refactors of allocation patterns (capacity-sized list construction, extracted LINQ filter method) with no new shared state.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 51.7 AIC · ⌖ 9.39 AIC · ⊞ 24.8K · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@Evangelink
Amaury Levé (Evangelink) merged commit 39bf3c0 into mainAug 24, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-servermode-notifications branch August 24, 2026 08:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[perf-improver] Avoid LINQ allocations on ServerMode per-notification hot path

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Reduce ServerMode notification allocations - #10670

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications
Aug 24, 2026
Merged

Reduce ServerMode notification allocations#10670
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • replace the TestNodeStateChangedEventArgs array LINQ projection with a capacity-sized List<object> fill while preserving null/empty behavior and runtime collection typing
  • isolate the aggregator's captured filtering predicate so batches without terminal states avoid its closure allocation
  • preserve the existing filtered LINQ materialization because pre-sized array/list alternatives increased allocation for large, heavily suppressed batches
  • add focused coverage for exact JSON shape/order, object typing, null and empty changes, retained identity/order, predicate behavior, and source immutability

Performance

A standalone faithful-shape harness measured median time and allocation over null, empty, 1, 8, 128, and 1,024 item inputs on .NET 8 and .NET Framework 4.6.2.

  • serializer, 8 items: 2,336 B -> 2,232 B on .NET 8; approximately 2,367 B -> 2,247 B on .NET Framework
  • serializer, 1,024 items: 286,984 B -> 278,584 B on .NET 8; approximately 287,857 B -> 279,413 B on .NET Framework
  • aggregator without terminal states: 24 B less allocation per batch on .NET 8, with the terminal/filtering path retaining its prior allocation shape

The serializer percentages are intentionally omitted because the harness models the container work faithfully but uses a smaller per-item payload than production. .NET Framework allocation values are approximate because its counter is AppDomain-wide.

Validation

.\build.cmd -projects .\test\UnitTests\Microsoft.Testing.Platform.UnitTests\Microsoft.Testing.Platform.UnitTests.csproj -test -bl

Passed on net8.0, net9.0, and net462 with 0 warnings and 0 errors.

Closes#10669

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 22, 2026 18:19
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 22, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Reduces ServerMode notification allocations while preserving serialization and aggregation behavior.

Changes:

  • Replaces serializer LINQ projection with a capacity-sized list.
  • Isolates the aggregator’s captured filtering predicate.
  • Adds serialization and aggregation regression tests.
Show a summary per file
FileDescription
SerializerUtilities.TestNodeSerializers.csOptimizes change serialization.
TestNodeStateChangeAggregator.csAvoids closure allocation for non-terminal batches.
FormatterUtilitiesTests.csTests serialization shape, typing, order, and null handling.
TestNodeStateChangeAggregatorTests.csTests repeated aggregation behavior.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1f616dbd-edac-4ae9-b58a-b0d7b4739297
CopilotAI review requested due to automatic review settings August 22, 2026 21:58
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10670

GradeTestMutationNotesHow to improve
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
DoesNotMutateBufferedChanges
4/4 killedMutates a buffered terminal item between two builds and asserts the previously suppressed in-progress item reappears in the right slot, proving the buffer itself is never mutated.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
Changes_
PreservesOrderObjectTypesAndSource
3/3 killedVerifies element order, UID values, and reference identity of the original source array against both the intermediate dictionary and the final JSON string, directly exercising the new indexed serialization loop.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
EmptyChanges_
PreservesEmptyObjectList
2/2 killedConfirms an empty (non-null) changes array serializes to [] rather than null, guarding the new null-check branch.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
NullChanges_
PreservesNull
2/2 killedConfirms a null changes array is preserved as null rather than becoming an empty list, guarding the new null-check branch.

Summary: This PR refactors TestNodeStateChangeAggregator.BuildAggregatedChange and the
TestNodeStateChangedEventArgs serializer to use an indexed loop instead of LINQ. All four new/
modified tests are well-targeted regression tests for the exact behaviors the refactor could
break (null vs. empty changes, element order/identity preservation, and buffer non-mutation
across repeated aggregation calls). No high-confidence actionable findings — all four tests
graded A with no inline suggestions.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 73.3 AIC · ⌖ 3.44 AIC · ⊞ 16.9K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10670

Parallelization — one row per test assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Platform.UnitTestsMethodLevel (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs)CPU countcoverable once the parallel-safety analyzers ship (attribute-based opt-in)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

Nothing to flag. Both changed test files add pure, instance-scoped test methods with no cross-test coordination concerns:

  • TestNodeStateChangeAggregatorTests.BuildAggregatedChange_DoesNotMutateBufferedChanges (new) creates its own local TestNodeStateChangeAggregator instance and only mutates a locally-owned TestNode.Properties._testNodeStateProperty on a TestNodeUpdateMessage it constructed itself in the test body — no shared/static state, no environment variables, no filesystem paths, no Console/culture mutation.
  • FormatterUtilitiesTests (new Serialize_TestNodeStateChangedEventArgs_* tests) exercise SerializerUtilities.Serialize against locally-constructed messages via the instance-level _formatter field created once per test-class instantiation (MSTest constructs the class per test). SerializerUtilities's backing Dictionary<Type, IObjectSerializer> fields are populated once in a static constructor and only read afterward by every test in the assembly — no test mutates them, so there is no cross-test write/write or read/write race even under MethodLevel.
  • No [ResourceLock], [DoNotParallelize], or assembly-level parallelization declaration was added, removed, or otherwise touched by this PR.

No production-code changes in this PR (SerializerUtilities.TestNodeSerializers.cs, TestNodeStateChangeAggregator.cs) introduce process-global mutation either — both are pure refactors of allocation patterns (capacity-sized list construction, extracted LINQ filter method) with no new shared state.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 51.7 AIC · ⌖ 9.39 AIC · ⊞ 24.8K · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@Evangelink
Amaury Levé (Evangelink) merged commit 39bf3c0 into mainAug 24, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-servermode-notifications branch August 24, 2026 08:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[perf-improver] Avoid LINQ allocations on ServerMode per-notification hot path

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Reduce ServerMode notification allocations - #10670

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications
Aug 24, 2026
Merged

Reduce ServerMode notification allocations#10670
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • replace the TestNodeStateChangedEventArgs array LINQ projection with a capacity-sized List<object> fill while preserving null/empty behavior and runtime collection typing
  • isolate the aggregator's captured filtering predicate so batches without terminal states avoid its closure allocation
  • preserve the existing filtered LINQ materialization because pre-sized array/list alternatives increased allocation for large, heavily suppressed batches
  • add focused coverage for exact JSON shape/order, object typing, null and empty changes, retained identity/order, predicate behavior, and source immutability

Performance

A standalone faithful-shape harness measured median time and allocation over null, empty, 1, 8, 128, and 1,024 item inputs on .NET 8 and .NET Framework 4.6.2.

  • serializer, 8 items: 2,336 B -> 2,232 B on .NET 8; approximately 2,367 B -> 2,247 B on .NET Framework
  • serializer, 1,024 items: 286,984 B -> 278,584 B on .NET 8; approximately 287,857 B -> 279,413 B on .NET Framework
  • aggregator without terminal states: 24 B less allocation per batch on .NET 8, with the terminal/filtering path retaining its prior allocation shape

The serializer percentages are intentionally omitted because the harness models the container work faithfully but uses a smaller per-item payload than production. .NET Framework allocation values are approximate because its counter is AppDomain-wide.

Validation

.\build.cmd -projects .\test\UnitTests\Microsoft.Testing.Platform.UnitTests\Microsoft.Testing.Platform.UnitTests.csproj -test -bl

Passed on net8.0, net9.0, and net462 with 0 warnings and 0 errors.

Closes#10669

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 22, 2026 18:19
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 22, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Reduces ServerMode notification allocations while preserving serialization and aggregation behavior.

Changes:

  • Replaces serializer LINQ projection with a capacity-sized list.
  • Isolates the aggregator’s captured filtering predicate.
  • Adds serialization and aggregation regression tests.
Show a summary per file
FileDescription
SerializerUtilities.TestNodeSerializers.csOptimizes change serialization.
TestNodeStateChangeAggregator.csAvoids closure allocation for non-terminal batches.
FormatterUtilitiesTests.csTests serialization shape, typing, order, and null handling.
TestNodeStateChangeAggregatorTests.csTests repeated aggregation behavior.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1f616dbd-edac-4ae9-b58a-b0d7b4739297
CopilotAI review requested due to automatic review settings August 22, 2026 21:58
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10670

GradeTestMutationNotesHow to improve
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
DoesNotMutateBufferedChanges
4/4 killedMutates a buffered terminal item between two builds and asserts the previously suppressed in-progress item reappears in the right slot, proving the buffer itself is never mutated.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
Changes_
PreservesOrderObjectTypesAndSource
3/3 killedVerifies element order, UID values, and reference identity of the original source array against both the intermediate dictionary and the final JSON string, directly exercising the new indexed serialization loop.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
EmptyChanges_
PreservesEmptyObjectList
2/2 killedConfirms an empty (non-null) changes array serializes to [] rather than null, guarding the new null-check branch.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
NullChanges_
PreservesNull
2/2 killedConfirms a null changes array is preserved as null rather than becoming an empty list, guarding the new null-check branch.

Summary: This PR refactors TestNodeStateChangeAggregator.BuildAggregatedChange and the
TestNodeStateChangedEventArgs serializer to use an indexed loop instead of LINQ. All four new/
modified tests are well-targeted regression tests for the exact behaviors the refactor could
break (null vs. empty changes, element order/identity preservation, and buffer non-mutation
across repeated aggregation calls). No high-confidence actionable findings — all four tests
graded A with no inline suggestions.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 73.3 AIC · ⌖ 3.44 AIC · ⊞ 16.9K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10670

Parallelization — one row per test assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Platform.UnitTestsMethodLevel (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs)CPU countcoverable once the parallel-safety analyzers ship (attribute-based opt-in)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

Nothing to flag. Both changed test files add pure, instance-scoped test methods with no cross-test coordination concerns:

  • TestNodeStateChangeAggregatorTests.BuildAggregatedChange_DoesNotMutateBufferedChanges (new) creates its own local TestNodeStateChangeAggregator instance and only mutates a locally-owned TestNode.Properties._testNodeStateProperty on a TestNodeUpdateMessage it constructed itself in the test body — no shared/static state, no environment variables, no filesystem paths, no Console/culture mutation.
  • FormatterUtilitiesTests (new Serialize_TestNodeStateChangedEventArgs_* tests) exercise SerializerUtilities.Serialize against locally-constructed messages via the instance-level _formatter field created once per test-class instantiation (MSTest constructs the class per test). SerializerUtilities's backing Dictionary<Type, IObjectSerializer> fields are populated once in a static constructor and only read afterward by every test in the assembly — no test mutates them, so there is no cross-test write/write or read/write race even under MethodLevel.
  • No [ResourceLock], [DoNotParallelize], or assembly-level parallelization declaration was added, removed, or otherwise touched by this PR.

No production-code changes in this PR (SerializerUtilities.TestNodeSerializers.cs, TestNodeStateChangeAggregator.cs) introduce process-global mutation either — both are pure refactors of allocation patterns (capacity-sized list construction, extracted LINQ filter method) with no new shared state.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 51.7 AIC · ⌖ 9.39 AIC · ⊞ 24.8K · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@Evangelink
Amaury Levé (Evangelink) merged commit 39bf3c0 into mainAug 24, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-servermode-notifications branch August 24, 2026 08:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[perf-improver] Avoid LINQ allocations on ServerMode per-notification hot path

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Reduce ServerMode notification allocations - #10670

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications
Aug 24, 2026
Merged

Reduce ServerMode notification allocations#10670
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • replace the TestNodeStateChangedEventArgs array LINQ projection with a capacity-sized List<object> fill while preserving null/empty behavior and runtime collection typing
  • isolate the aggregator's captured filtering predicate so batches without terminal states avoid its closure allocation
  • preserve the existing filtered LINQ materialization because pre-sized array/list alternatives increased allocation for large, heavily suppressed batches
  • add focused coverage for exact JSON shape/order, object typing, null and empty changes, retained identity/order, predicate behavior, and source immutability

Performance

A standalone faithful-shape harness measured median time and allocation over null, empty, 1, 8, 128, and 1,024 item inputs on .NET 8 and .NET Framework 4.6.2.

  • serializer, 8 items: 2,336 B -> 2,232 B on .NET 8; approximately 2,367 B -> 2,247 B on .NET Framework
  • serializer, 1,024 items: 286,984 B -> 278,584 B on .NET 8; approximately 287,857 B -> 279,413 B on .NET Framework
  • aggregator without terminal states: 24 B less allocation per batch on .NET 8, with the terminal/filtering path retaining its prior allocation shape

The serializer percentages are intentionally omitted because the harness models the container work faithfully but uses a smaller per-item payload than production. .NET Framework allocation values are approximate because its counter is AppDomain-wide.

Validation

.\build.cmd -projects .\test\UnitTests\Microsoft.Testing.Platform.UnitTests\Microsoft.Testing.Platform.UnitTests.csproj -test -bl

Passed on net8.0, net9.0, and net462 with 0 warnings and 0 errors.

Closes#10669

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 22, 2026 18:19
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 22, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Reduces ServerMode notification allocations while preserving serialization and aggregation behavior.

Changes:

  • Replaces serializer LINQ projection with a capacity-sized list.
  • Isolates the aggregator’s captured filtering predicate.
  • Adds serialization and aggregation regression tests.
Show a summary per file
FileDescription
SerializerUtilities.TestNodeSerializers.csOptimizes change serialization.
TestNodeStateChangeAggregator.csAvoids closure allocation for non-terminal batches.
FormatterUtilitiesTests.csTests serialization shape, typing, order, and null handling.
TestNodeStateChangeAggregatorTests.csTests repeated aggregation behavior.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1f616dbd-edac-4ae9-b58a-b0d7b4739297
CopilotAI review requested due to automatic review settings August 22, 2026 21:58
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10670

GradeTestMutationNotesHow to improve
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
DoesNotMutateBufferedChanges
4/4 killedMutates a buffered terminal item between two builds and asserts the previously suppressed in-progress item reappears in the right slot, proving the buffer itself is never mutated.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
Changes_
PreservesOrderObjectTypesAndSource
3/3 killedVerifies element order, UID values, and reference identity of the original source array against both the intermediate dictionary and the final JSON string, directly exercising the new indexed serialization loop.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
EmptyChanges_
PreservesEmptyObjectList
2/2 killedConfirms an empty (non-null) changes array serializes to [] rather than null, guarding the new null-check branch.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
NullChanges_
PreservesNull
2/2 killedConfirms a null changes array is preserved as null rather than becoming an empty list, guarding the new null-check branch.

Summary: This PR refactors TestNodeStateChangeAggregator.BuildAggregatedChange and the
TestNodeStateChangedEventArgs serializer to use an indexed loop instead of LINQ. All four new/
modified tests are well-targeted regression tests for the exact behaviors the refactor could
break (null vs. empty changes, element order/identity preservation, and buffer non-mutation
across repeated aggregation calls). No high-confidence actionable findings — all four tests
graded A with no inline suggestions.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 73.3 AIC · ⌖ 3.44 AIC · ⊞ 16.9K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10670

Parallelization — one row per test assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Platform.UnitTestsMethodLevel (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs)CPU countcoverable once the parallel-safety analyzers ship (attribute-based opt-in)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

Nothing to flag. Both changed test files add pure, instance-scoped test methods with no cross-test coordination concerns:

  • TestNodeStateChangeAggregatorTests.BuildAggregatedChange_DoesNotMutateBufferedChanges (new) creates its own local TestNodeStateChangeAggregator instance and only mutates a locally-owned TestNode.Properties._testNodeStateProperty on a TestNodeUpdateMessage it constructed itself in the test body — no shared/static state, no environment variables, no filesystem paths, no Console/culture mutation.
  • FormatterUtilitiesTests (new Serialize_TestNodeStateChangedEventArgs_* tests) exercise SerializerUtilities.Serialize against locally-constructed messages via the instance-level _formatter field created once per test-class instantiation (MSTest constructs the class per test). SerializerUtilities's backing Dictionary<Type, IObjectSerializer> fields are populated once in a static constructor and only read afterward by every test in the assembly — no test mutates them, so there is no cross-test write/write or read/write race even under MethodLevel.
  • No [ResourceLock], [DoNotParallelize], or assembly-level parallelization declaration was added, removed, or otherwise touched by this PR.

No production-code changes in this PR (SerializerUtilities.TestNodeSerializers.cs, TestNodeStateChangeAggregator.cs) introduce process-global mutation either — both are pure refactors of allocation patterns (capacity-sized list construction, extracted LINQ filter method) with no new shared state.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 51.7 AIC · ⌖ 9.39 AIC · ⊞ 24.8K · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@Evangelink
Amaury Levé (Evangelink) merged commit 39bf3c0 into mainAug 24, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-servermode-notifications branch August 24, 2026 08:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[perf-improver] Avoid LINQ allocations on ServerMode per-notification hot path

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Reduce ServerMode notification allocations - #10670

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications
Aug 24, 2026
Merged

Reduce ServerMode notification allocations#10670
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • replace the TestNodeStateChangedEventArgs array LINQ projection with a capacity-sized List<object> fill while preserving null/empty behavior and runtime collection typing
  • isolate the aggregator's captured filtering predicate so batches without terminal states avoid its closure allocation
  • preserve the existing filtered LINQ materialization because pre-sized array/list alternatives increased allocation for large, heavily suppressed batches
  • add focused coverage for exact JSON shape/order, object typing, null and empty changes, retained identity/order, predicate behavior, and source immutability

Performance

A standalone faithful-shape harness measured median time and allocation over null, empty, 1, 8, 128, and 1,024 item inputs on .NET 8 and .NET Framework 4.6.2.

  • serializer, 8 items: 2,336 B -> 2,232 B on .NET 8; approximately 2,367 B -> 2,247 B on .NET Framework
  • serializer, 1,024 items: 286,984 B -> 278,584 B on .NET 8; approximately 287,857 B -> 279,413 B on .NET Framework
  • aggregator without terminal states: 24 B less allocation per batch on .NET 8, with the terminal/filtering path retaining its prior allocation shape

The serializer percentages are intentionally omitted because the harness models the container work faithfully but uses a smaller per-item payload than production. .NET Framework allocation values are approximate because its counter is AppDomain-wide.

Validation

.\build.cmd -projects .\test\UnitTests\Microsoft.Testing.Platform.UnitTests\Microsoft.Testing.Platform.UnitTests.csproj -test -bl

Passed on net8.0, net9.0, and net462 with 0 warnings and 0 errors.

Closes#10669

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 22, 2026 18:19
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 22, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Reduces ServerMode notification allocations while preserving serialization and aggregation behavior.

Changes:

  • Replaces serializer LINQ projection with a capacity-sized list.
  • Isolates the aggregator’s captured filtering predicate.
  • Adds serialization and aggregation regression tests.
Show a summary per file
FileDescription
SerializerUtilities.TestNodeSerializers.csOptimizes change serialization.
TestNodeStateChangeAggregator.csAvoids closure allocation for non-terminal batches.
FormatterUtilitiesTests.csTests serialization shape, typing, order, and null handling.
TestNodeStateChangeAggregatorTests.csTests repeated aggregation behavior.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1f616dbd-edac-4ae9-b58a-b0d7b4739297
CopilotAI review requested due to automatic review settings August 22, 2026 21:58
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10670

GradeTestMutationNotesHow to improve
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
DoesNotMutateBufferedChanges
4/4 killedMutates a buffered terminal item between two builds and asserts the previously suppressed in-progress item reappears in the right slot, proving the buffer itself is never mutated.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
Changes_
PreservesOrderObjectTypesAndSource
3/3 killedVerifies element order, UID values, and reference identity of the original source array against both the intermediate dictionary and the final JSON string, directly exercising the new indexed serialization loop.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
EmptyChanges_
PreservesEmptyObjectList
2/2 killedConfirms an empty (non-null) changes array serializes to [] rather than null, guarding the new null-check branch.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
NullChanges_
PreservesNull
2/2 killedConfirms a null changes array is preserved as null rather than becoming an empty list, guarding the new null-check branch.

Summary: This PR refactors TestNodeStateChangeAggregator.BuildAggregatedChange and the
TestNodeStateChangedEventArgs serializer to use an indexed loop instead of LINQ. All four new/
modified tests are well-targeted regression tests for the exact behaviors the refactor could
break (null vs. empty changes, element order/identity preservation, and buffer non-mutation
across repeated aggregation calls). No high-confidence actionable findings — all four tests
graded A with no inline suggestions.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 73.3 AIC · ⌖ 3.44 AIC · ⊞ 16.9K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10670

Parallelization — one row per test assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Platform.UnitTestsMethodLevel (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs)CPU countcoverable once the parallel-safety analyzers ship (attribute-based opt-in)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

Nothing to flag. Both changed test files add pure, instance-scoped test methods with no cross-test coordination concerns:

  • TestNodeStateChangeAggregatorTests.BuildAggregatedChange_DoesNotMutateBufferedChanges (new) creates its own local TestNodeStateChangeAggregator instance and only mutates a locally-owned TestNode.Properties._testNodeStateProperty on a TestNodeUpdateMessage it constructed itself in the test body — no shared/static state, no environment variables, no filesystem paths, no Console/culture mutation.
  • FormatterUtilitiesTests (new Serialize_TestNodeStateChangedEventArgs_* tests) exercise SerializerUtilities.Serialize against locally-constructed messages via the instance-level _formatter field created once per test-class instantiation (MSTest constructs the class per test). SerializerUtilities's backing Dictionary<Type, IObjectSerializer> fields are populated once in a static constructor and only read afterward by every test in the assembly — no test mutates them, so there is no cross-test write/write or read/write race even under MethodLevel.
  • No [ResourceLock], [DoNotParallelize], or assembly-level parallelization declaration was added, removed, or otherwise touched by this PR.

No production-code changes in this PR (SerializerUtilities.TestNodeSerializers.cs, TestNodeStateChangeAggregator.cs) introduce process-global mutation either — both are pure refactors of allocation patterns (capacity-sized list construction, extracted LINQ filter method) with no new shared state.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 51.7 AIC · ⌖ 9.39 AIC · ⊞ 24.8K · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@Evangelink
Amaury Levé (Evangelink) merged commit 39bf3c0 into mainAug 24, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-servermode-notifications branch August 24, 2026 08:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[perf-improver] Avoid LINQ allocations on ServerMode per-notification hot path

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Reduce ServerMode notification allocations - #10670

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications
Aug 24, 2026
Merged

Reduce ServerMode notification allocations#10670
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • replace the TestNodeStateChangedEventArgs array LINQ projection with a capacity-sized List<object> fill while preserving null/empty behavior and runtime collection typing
  • isolate the aggregator's captured filtering predicate so batches without terminal states avoid its closure allocation
  • preserve the existing filtered LINQ materialization because pre-sized array/list alternatives increased allocation for large, heavily suppressed batches
  • add focused coverage for exact JSON shape/order, object typing, null and empty changes, retained identity/order, predicate behavior, and source immutability

Performance

A standalone faithful-shape harness measured median time and allocation over null, empty, 1, 8, 128, and 1,024 item inputs on .NET 8 and .NET Framework 4.6.2.

  • serializer, 8 items: 2,336 B -> 2,232 B on .NET 8; approximately 2,367 B -> 2,247 B on .NET Framework
  • serializer, 1,024 items: 286,984 B -> 278,584 B on .NET 8; approximately 287,857 B -> 279,413 B on .NET Framework
  • aggregator without terminal states: 24 B less allocation per batch on .NET 8, with the terminal/filtering path retaining its prior allocation shape

The serializer percentages are intentionally omitted because the harness models the container work faithfully but uses a smaller per-item payload than production. .NET Framework allocation values are approximate because its counter is AppDomain-wide.

Validation

.\build.cmd -projects .\test\UnitTests\Microsoft.Testing.Platform.UnitTests\Microsoft.Testing.Platform.UnitTests.csproj -test -bl

Passed on net8.0, net9.0, and net462 with 0 warnings and 0 errors.

Closes#10669

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 22, 2026 18:19
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 22, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Reduces ServerMode notification allocations while preserving serialization and aggregation behavior.

Changes:

  • Replaces serializer LINQ projection with a capacity-sized list.
  • Isolates the aggregator’s captured filtering predicate.
  • Adds serialization and aggregation regression tests.
Show a summary per file
FileDescription
SerializerUtilities.TestNodeSerializers.csOptimizes change serialization.
TestNodeStateChangeAggregator.csAvoids closure allocation for non-terminal batches.
FormatterUtilitiesTests.csTests serialization shape, typing, order, and null handling.
TestNodeStateChangeAggregatorTests.csTests repeated aggregation behavior.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1f616dbd-edac-4ae9-b58a-b0d7b4739297
CopilotAI review requested due to automatic review settings August 22, 2026 21:58
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10670

GradeTestMutationNotesHow to improve
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
DoesNotMutateBufferedChanges
4/4 killedMutates a buffered terminal item between two builds and asserts the previously suppressed in-progress item reappears in the right slot, proving the buffer itself is never mutated.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
Changes_
PreservesOrderObjectTypesAndSource
3/3 killedVerifies element order, UID values, and reference identity of the original source array against both the intermediate dictionary and the final JSON string, directly exercising the new indexed serialization loop.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
EmptyChanges_
PreservesEmptyObjectList
2/2 killedConfirms an empty (non-null) changes array serializes to [] rather than null, guarding the new null-check branch.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
NullChanges_
PreservesNull
2/2 killedConfirms a null changes array is preserved as null rather than becoming an empty list, guarding the new null-check branch.

Summary: This PR refactors TestNodeStateChangeAggregator.BuildAggregatedChange and the
TestNodeStateChangedEventArgs serializer to use an indexed loop instead of LINQ. All four new/
modified tests are well-targeted regression tests for the exact behaviors the refactor could
break (null vs. empty changes, element order/identity preservation, and buffer non-mutation
across repeated aggregation calls). No high-confidence actionable findings — all four tests
graded A with no inline suggestions.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 73.3 AIC · ⌖ 3.44 AIC · ⊞ 16.9K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10670

Parallelization — one row per test assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Platform.UnitTestsMethodLevel (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs)CPU countcoverable once the parallel-safety analyzers ship (attribute-based opt-in)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

Nothing to flag. Both changed test files add pure, instance-scoped test methods with no cross-test coordination concerns:

  • TestNodeStateChangeAggregatorTests.BuildAggregatedChange_DoesNotMutateBufferedChanges (new) creates its own local TestNodeStateChangeAggregator instance and only mutates a locally-owned TestNode.Properties._testNodeStateProperty on a TestNodeUpdateMessage it constructed itself in the test body — no shared/static state, no environment variables, no filesystem paths, no Console/culture mutation.
  • FormatterUtilitiesTests (new Serialize_TestNodeStateChangedEventArgs_* tests) exercise SerializerUtilities.Serialize against locally-constructed messages via the instance-level _formatter field created once per test-class instantiation (MSTest constructs the class per test). SerializerUtilities's backing Dictionary<Type, IObjectSerializer> fields are populated once in a static constructor and only read afterward by every test in the assembly — no test mutates them, so there is no cross-test write/write or read/write race even under MethodLevel.
  • No [ResourceLock], [DoNotParallelize], or assembly-level parallelization declaration was added, removed, or otherwise touched by this PR.

No production-code changes in this PR (SerializerUtilities.TestNodeSerializers.cs, TestNodeStateChangeAggregator.cs) introduce process-global mutation either — both are pure refactors of allocation patterns (capacity-sized list construction, extracted LINQ filter method) with no new shared state.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 51.7 AIC · ⌖ 9.39 AIC · ⊞ 24.8K · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@Evangelink
Amaury Levé (Evangelink) merged commit 39bf3c0 into mainAug 24, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-servermode-notifications branch August 24, 2026 08:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[perf-improver] Avoid LINQ allocations on ServerMode per-notification hot path

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Reduce ServerMode notification allocations - #10670

Merged
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications
Aug 24, 2026
Merged

Reduce ServerMode notification allocations#10670
Amaury Levé (Evangelink) merged 2 commits into
mainfrom
dev/amauryleve/optimize-servermode-notifications

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

  • replace the TestNodeStateChangedEventArgs array LINQ projection with a capacity-sized List<object> fill while preserving null/empty behavior and runtime collection typing
  • isolate the aggregator's captured filtering predicate so batches without terminal states avoid its closure allocation
  • preserve the existing filtered LINQ materialization because pre-sized array/list alternatives increased allocation for large, heavily suppressed batches
  • add focused coverage for exact JSON shape/order, object typing, null and empty changes, retained identity/order, predicate behavior, and source immutability

Performance

A standalone faithful-shape harness measured median time and allocation over null, empty, 1, 8, 128, and 1,024 item inputs on .NET 8 and .NET Framework 4.6.2.

  • serializer, 8 items: 2,336 B -> 2,232 B on .NET 8; approximately 2,367 B -> 2,247 B on .NET Framework
  • serializer, 1,024 items: 286,984 B -> 278,584 B on .NET 8; approximately 287,857 B -> 279,413 B on .NET Framework
  • aggregator without terminal states: 24 B less allocation per batch on .NET 8, with the terminal/filtering path retaining its prior allocation shape

The serializer percentages are intentionally omitted because the harness models the container work faithfully but uses a smaller per-item payload than production. .NET Framework allocation values are approximate because its counter is AppDomain-wide.

Validation

.\build.cmd -projects .\test\UnitTests\Microsoft.Testing.Platform.UnitTests\Microsoft.Testing.Platform.UnitTests.csproj -test -bl

Passed on net8.0, net9.0, and net462 with 0 warnings and 0 errors.

Closes#10669

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI balanced review requested due to automatic review settings August 22, 2026 18:19
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Aug 22, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Reduces ServerMode notification allocations while preserving serialization and aggregation behavior.

Changes:

  • Replaces serializer LINQ projection with a capacity-sized list.
  • Isolates the aggregator’s captured filtering predicate.
  • Adds serialization and aggregation regression tests.
Show a summary per file
FileDescription
SerializerUtilities.TestNodeSerializers.csOptimizes change serialization.
TestNodeStateChangeAggregator.csAvoids closure allocation for non-terminal batches.
FormatterUtilitiesTests.csTests serialization shape, typing, order, and null handling.
TestNodeStateChangeAggregatorTests.csTests repeated aggregation behavior.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1f616dbd-edac-4ae9-b58a-b0d7b4739297
CopilotAI review requested due to automatic review settings August 22, 2026 21:58
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #10670

GradeTestMutationNotesHow to improve
A (90–100)new TestNodeStateChangeAggregatorTests.
BuildAggregatedChange_
DoesNotMutateBufferedChanges
4/4 killedMutates a buffered terminal item between two builds and asserts the previously suppressed in-progress item reappears in the right slot, proving the buffer itself is never mutated.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
Changes_
PreservesOrderObjectTypesAndSource
3/3 killedVerifies element order, UID values, and reference identity of the original source array against both the intermediate dictionary and the final JSON string, directly exercising the new indexed serialization loop.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
EmptyChanges_
PreservesEmptyObjectList
2/2 killedConfirms an empty (non-null) changes array serializes to [] rather than null, guarding the new null-check branch.
A (90–100)new FormatterUtilitiesTests.
Serialize_
TestNodeStateChangedEventArgs_
NullChanges_
PreservesNull
2/2 killedConfirms a null changes array is preserved as null rather than becoming an empty list, guarding the new null-check branch.

Summary: This PR refactors TestNodeStateChangeAggregator.BuildAggregatedChange and the
TestNodeStateChangedEventArgs serializer to use an indexed loop instead of LINQ. All four new/
modified tests are well-targeted regression tests for the exact behaviors the refactor could
break (null vs. empty changes, element order/identity preservation, and buffer non-mutation
across repeated aggregation calls). No high-confidence actionable findings — all four tests
graded A with no inline suggestions.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 73.3 AIC · ⌖ 3.44 AIC · ⊞ 16.9K · [◷]( · )

@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #10670

Parallelization — one row per test assembly audited:

Test assemblyScopeWorkersAnalyzer coverage
Microsoft.Testing.Platform.UnitTestsMethodLevel (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs)CPU countcoverable once the parallel-safety analyzers ship (attribute-based opt-in)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

Nothing to flag. Both changed test files add pure, instance-scoped test methods with no cross-test coordination concerns:

  • TestNodeStateChangeAggregatorTests.BuildAggregatedChange_DoesNotMutateBufferedChanges (new) creates its own local TestNodeStateChangeAggregator instance and only mutates a locally-owned TestNode.Properties._testNodeStateProperty on a TestNodeUpdateMessage it constructed itself in the test body — no shared/static state, no environment variables, no filesystem paths, no Console/culture mutation.
  • FormatterUtilitiesTests (new Serialize_TestNodeStateChangedEventArgs_* tests) exercise SerializerUtilities.Serialize against locally-constructed messages via the instance-level _formatter field created once per test-class instantiation (MSTest constructs the class per test). SerializerUtilities's backing Dictionary<Type, IObjectSerializer> fields are populated once in a static constructor and only read afterward by every test in the assembly — no test mutates them, so there is no cross-test write/write or read/write race even under MethodLevel.
  • No [ResourceLock], [DoNotParallelize], or assembly-level parallelization declaration was added, removed, or otherwise touched by this PR.

No production-code changes in this PR (SerializerUtilities.TestNodeSerializers.cs, TestNodeStateChangeAggregator.cs) introduce process-global mutation either — both are pure refactors of allocation patterns (capacity-sized list construction, extracted LINQ filter method) with no new shared state.

Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 51.7 AIC · ⌖ 9.39 AIC · ⊞ 24.8K · [◷]( · )

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@Evangelink
Amaury Levé (Evangelink) merged commit 39bf3c0 into mainAug 24, 2026
46 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/optimize-servermode-notifications branch August 24, 2026 08:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[perf-improver] Avoid LINQ allocations on ServerMode per-notification hot path

3 participants

@Evangelink@0101