Skip to content
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

Revert "RID: Change comparison operators to use RID_Data id instead of address" #69946

Merged
merged 1 commit into from
Dec 12, 2022

Conversation

akien-mga
Copy link
Member

See #59234 (comment) and #67273 for a description of the problem it causes.

The RID comparisons should be avoided as far as possible - there's only a few of them (see graph in #59234) and we should aim to make them more reliable in a safer way.

@akien-mga akien-mga added bug topic:core regression cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release labels Dec 12, 2022
@akien-mga akien-mga added this to the 3.6 milestone Dec 12, 2022
@akien-mga akien-mga requested a review from a team as a code owner December 12, 2022 08:12
@lawnjelly
Copy link
Member

Just for reference, we decided where order wasn't important, the existing data pointer was better as there is no lookup (hence no cache misses, and no dangling RID problems), and where order matters, RIDs are not an appropriate ordering index, and we should revisit these few areas and fix them individually (especially canvas items).

@akien-mga akien-mga merged commit 9ef525c into 3.x Dec 12, 2022
@akien-mga akien-mga deleted the revert-59234-3.x-rid-comparison-id branch December 12, 2022 09:23
@akien-mga
Copy link
Member Author

Cherry-picked for 3.5.2.

@akien-mga akien-mga removed the cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release label Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants