Skip to content

Conversation

@rekhoff
Copy link
Contributor

@rekhoff rekhoff commented Jan 3, 2026

Description of Changes

This PR fixes a C# codegen performance/behavior issue triggered by views that include nullable value-type fields (e.g. DbVector2?), as reported in #3914.

  • Updated the C# BSATN code generator to emit non-boxing equality for Nullable<T> members by using System.Nullable.Equals(a, b) rather than a.Equals(b) (which can box and cause excessive host logging/work in view diff/equality paths).

This causes code generation to change from something like:

// From:
var ___eqNullableIntField = this.NullableIntField.Equals(that.NullableIntField);
// To:
var ___eqNullableIntField = System.Nullable.Equals(
    this.NullableIntField,
    that.NullableIntField
);
  • Added a regression scenario to the C# regression-test module that exercises the problematic pattern:
    • A table containing a nullable struct field (DbVector2? Pos).
    • A public view that returns rows containing that nullable field.
    • A reducer to mutate the nullable field from somenonesome to force view re-evaluation and diffing.
  • Updated the regression-test client to:
    • Subscribe to the new view.
    • Validate that the view evaluates successfully and contains the expected rows.
    • Call the reducer and validate view state after updates.
    • Fail the test if view evaluation/diffing produces errors.

API and ABI breaking changes

None.

  • No changes to public SpacetimeDB schema or wire format semantics.
  • Changes are limited to generated equality code and regression tests.

Expected complexity level and risk

2 - Low

  • The codegen change is small and localized (special-casing System.Nullable<T> equality generation).
  • Risk is primarily around subtle behavior differences in equality for nullable value types; however, System.Nullable.Equals matches the expected semantics and avoids boxing.
  • Regression tests specifically cover the nullable-struct-in-view scenario to guard against regressions.

Testing

  • Ran the C# regression test suite via run-regression-tests.sh and confirmed no errors.

@rekhoff rekhoff self-assigned this Jan 3, 2026
@rekhoff rekhoff marked this pull request as ready for review January 5, 2026 17:30
Copy link
Contributor

@JasonAtClockwork JasonAtClockwork left a comment

Choose a reason for hiding this comment

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

Solid improvement with additional testing! I ran the tests as well with no difficulties.

@rekhoff rekhoff added this pull request to the merge queue Jan 6, 2026
Merged via the queue into master with commit f906be6 Jan 6, 2026
27 checks passed
@rekhoff rekhoff deleted the rekhoff/view-nullable-type-fix branch January 6, 2026 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants