Skip to content

Conversation

@nvborisenko
Copy link
Member

Improve performance for commands/events internally.

💥 What does this PR do?

This pull request refactors the internal event and command handling logic in the Broker class to simplify data structures and improve type safety. The main changes involve replacing tuple-based and class-based representations with record struct types for commands and events, and updating related logic accordingly.

🔄 Types of changes

  • Cleanup (formatting, renaming)

Copilot AI review requested due to automatic review settings February 11, 2026 10:24
@qodo-code-review
Copy link
Contributor

PR Type

Enhancement


Description

  • Refactor CommandInfo and EventInfo to record struct types

  • Remove redundant Id field from CommandInfo struct

  • Update _pendingEvents channel to use EventInfo struct

  • Improve performance through value type semantics


File Walkthrough

Relevant files
Enhancement
Broker.cs
Convert CommandInfo and EventInfo to record structs           

dotnet/src/webdriver/BiDi/Broker.cs

  • Converted CommandInfo class to private readonly record struct removing
    the Id property
  • Converted tuple-based event representation to EventInfo record struct
  • Updated _pendingEvents channel type from tuple to EventInfo struct
  • Simplified command info instantiation by removing command.Id parameter
+5/-11   

@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Feb 11, 2026
@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Unchecked channel write: The new _pendingEvents.Writer.TryWrite(new EventInfo(method, eventArgs)) does not check
the return value, which can silently drop events if the channel is completed or otherwise
unable to accept writes.

Referred Code
    _pendingEvents.Writer.TryWrite(new EventInfo(method, eventArgs));
}

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Revert struct to class to prevent potential null dereferences

Revert CommandInfo from a readonly record struct to a readonly record class to
prevent potential NullReferenceException issues when used with
ConcurrentDictionary and improve robustness.

dotnet/src/webdriver/BiDi/Broker.cs [323]

-private readonly record struct CommandInfo(TaskCompletionSource<EmptyResult> TaskCompletionSource, JsonTypeInfo JsonResultTypeInfo);
+private readonly record class CommandInfo(TaskCompletionSource<EmptyResult> TaskCompletionSource, JsonTypeInfo JsonResultTypeInfo);
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential risk of using a struct with reference-type fields in a dictionary, which could lead to NullReferenceException if not handled carefully. Although the current code uses TryRemove safely, changing back to a class makes the design more robust and less prone to future errors.

Low
  • More

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the internal BiDi Broker bookkeeping types to reduce allocations and simplify the representation of pending commands/events by using record struct values instead of tuples/classes.

Changes:

  • Replaced the pending event tuple channel with a strongly-typed EventInfo record struct channel.
  • Replaced CommandInfo reference type with a readonly record struct, removing the redundant stored command id (dictionary key already provides it).
  • Updated enqueue/dequeue logic to use the new struct types.

@nvborisenko nvborisenko merged commit abc4eaa into SeleniumHQ:trunk Feb 11, 2026
28 checks passed
@nvborisenko nvborisenko deleted the bidi-broker-struct-info branch February 11, 2026 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants