-
Notifications
You must be signed in to change notification settings - Fork 665
DYN-6732: Fix 'ReferenceEqualityComparer' hash collisions #16754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-6732
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a critical performance issue in CLRObjectMarshaler where dictionary lookups degraded from O(1) to O(n) when marshaling large numbers of geometry objects with identical coordinate values. The root cause was hash collisions in ReferenceEqualityComparer, which used value-based hash codes instead of identity-based hash codes.
Key Changes:
- Modified
ReferenceEqualityComparer.GetHashCode()to useRuntimeHelpers.GetHashCode(obj)instead ofobj.GetHashCode() - Updated XML documentation to clarify the comparer's purpose and hash code behavior
- Added
using System.Runtime.CompilerServicesdirective
Performance Impact:
- 425x speedup for marshaling 100K Point objects with identical coordinates (67,547ms → 159ms)
- Hash collision rate reduced from 100% to ~0.01%
aparajit-pratap
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benglin I'm curious if the GetHashCode implementation should be changed in classes like Point.cs as well.
aparajit-pratap
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benglin can you add a unit test for this? Thank you for the fix!
Ah no @aparajit-pratap,
Sure, I will be adding tests within |
…int values' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Purpose
Fixes severe performance degradation when marshaling large numbers of geometry objects with identical coordinate values (e.g., 100,000 Point objects at the same location). The issue was caused by hash collisions in
CLRObjectMapdictionary lookups, resulting in O(n) performance instead of O(1).Performance Impact
Root Cause:
ReferenceEqualityComparer.GetHashCode()was usingobj.GetHashCode(), which for geometry types likePointcomputes hash codes based on coordinate values. When multiple objects have identical coordinates, they produce identical hash codes, causing 100% collision rate and forcing the dictionary to traverse long collision chains.For example,
Point.ComputeHashCode()in LibG computes hash codes based on coordinate values:When 100,000
Pointobjects all have identical coordinates (e.g.,(0, 0, 0)), they all produce the same hash code, causing every dictionary lookup to traverse the entire collision chain, degrading performance from O(1) to O(n).Solution: Changed
ReferenceEqualityComparer.GetHashCode()to useRuntimeHelpers.GetHashCode(obj), which returns the object's identity hash code. This ensures well-distributed hash codes that match the reference equality semantics already used byReferenceEquals().Impact: The fix applies to all geometry types (Point, Line, Surface, Solid, etc.) and any other objects that may have value-based hash codes that collide.
Safety Note: This fix is safe and does not affect object equality semantics in Dynamo. The
CLRObjectMapdictionary already uses reference equality (object.ReferenceEquals) for comparisons, meaning twoPointobjects with identical coordinates but different instances are treated as different objects. The fix aligns the hash code computation with this existing reference equality behavior, ensuring consistent semantics while eliminating performance-degrading collisions.Testing
Declarations
Check these if you believe they are true
Release Notes
Fixed severe performance degradation when marshaling large numbers of geometry objects with identical coordinate values. The fix improves dictionary lookup performance by using identity-based hash codes that match reference equality semantics, eliminating hash collisions that caused O(n) lookup performance.
Reviewers
(Reviewer to be assigned)
Additional Notes
Technical Details:
ReferenceEqualityComparer.GetHashCode()inCLRObjectMarshaler.csto useRuntimeHelpers.GetHashCode(obj)instead ofobj.GetHashCode()object.ReferenceEquals)FYIs
(Optional) Names of anyone else you wish to be notified of