Skip to content

Avoid sorting CallableCustomMethodPointers by their actual address values#72346

Merged
YuriSizov merged 1 commit intogodotengine:masterfrom
myaaaaaaaaa:disconnect-order
Jul 26, 2023
Merged

Avoid sorting CallableCustomMethodPointers by their actual address values#72346
YuriSizov merged 1 commit intogodotengine:masterfrom
myaaaaaaaaa:disconnect-order

Conversation

@myaaaaaaaaa
Copy link
Copy Markdown
Contributor

#69874 is caused by signals being stored inside a VMap<Callable, ...>. This means that Callables that are allocated later would have a larger address and end up being sorted to the end of the VMap, making them easier to push/pop... until the C runtime starts recycling addresses, and connecting/disconnecting lots of signals becomes inexplicably slow.

This patch "fixes" that problem in that it makes connecting/disconnecting consistently slow, so that signal performance problems become easier to diagnose.

@myaaaaaaaaa myaaaaaaaaa requested review from a team as code owners January 30, 2023 02:13
@myaaaaaaaaa
Copy link
Copy Markdown
Contributor Author

Switched to a cleaner fix that just uses memcmp, to remove the need for the ifdef.

Because of how byte ordering works on little-endian machines, this will cause pointer digits to be compared from right to left, so they will no longer be sorted in a meaningful order.

@myaaaaaaaaa myaaaaaaaaa changed the title Sort CallableCustomMethodPointers by hash to avoid performance dependency on allocation order Avoid sorting CallableCustomMethodPointers by their actual address values Feb 9, 2023
@akien-mga akien-mga requested a review from RandomShaper June 19, 2023 20:47
@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 19, 2023
Copy link
Copy Markdown
Member

@RandomShaper RandomShaper left a comment

Choose a reason for hiding this comment

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

My "backwords" wasn't a typo, but, fun enough, "backwards" means the same in this context. 😃

@myaaaaaaaaa
Copy link
Copy Markdown
Contributor Author

myaaaaaaaaa commented Jun 20, 2023

My "backwords" wasn't a typo, but, fun enough, "backwards" means the same in this context. 😃

It failed CI, so I had no choice 🙃

@YuriSizov YuriSizov merged commit 53ba9cc into godotengine:master Jul 26, 2023
@YuriSizov
Copy link
Copy Markdown
Contributor

Thanks!

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.

4 participants