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

ActorMap, avoid IPositionable trait lookup. #20891

Merged

Conversation

anvilvapre
Copy link
Contributor

partial successor of #20351, which will be closed.

avoid trait dictionary lookup of IPositionable - by rather try to cast Actor.OccupiesSpace.

lookup up the trait in the trait dictionary requires a binary search on the actor in a list of all actors.

@Mailaender
Copy link
Member

Mailaender commented May 27, 2023

CPU

bleed: ⌀ 2.56 ± 1.74 ms
this: ⌀ 2.68 ± 1,76 ms

image

No adverse side effects: probably well inside the error margin.

Mailaender
Mailaender previously approved these changes May 27, 2023
@Mailaender
Copy link
Member

There are more occurrences of this trait lookup.

@anvilvapre anvilvapre force-pushed the 20230529_actor_map_ipositionable branch from 1f80196 to 5707c23 Compare June 1, 2023 11:56
@anvilvapre anvilvapre force-pushed the 20230529_actor_map_ipositionable branch from 5707c23 to 0290111 Compare June 1, 2023 16:28
Copy link
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

LGTM, untested

@Mailaender
Copy link
Member

Mailaender commented Jun 2, 2023

CPU

bleed: ⌀ 2.56 ± 1.74 ms
this: ⌀ 2.56 ± 1.74 ms

image

No adverse side effects, but also no improvement.

@Mailaender Mailaender removed their request for review June 2, 2023 09:47
@Mailaender Mailaender merged commit 7c0f6ea into OpenRA:bleed Jun 3, 2023
3 checks passed
@Mailaender
Copy link
Member

Changelog

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.

None yet

4 participants