Skip to content

Conversation

@keveleigh
Copy link
Contributor

@keveleigh keveleigh commented Nov 12, 2021

Overview

While digging into #10310, I noticed a couple spots where GetComponent on objects without the corresponding component were both costly and caused allocations. Unity 2019 introduced TryGetComponent, which helps reduce those side effects.

Before:
image
image

After:
image

@keveleigh keveleigh added this to the MRTK 2.x future milestone Nov 12, 2021
@keveleigh keveleigh requested a review from CDiaz-MS as a code owner November 12, 2021 02:08
@keveleigh keveleigh self-assigned this Nov 12, 2021
@keveleigh keveleigh requested a review from julenka as a code owner November 12, 2021 02:08
@keveleigh
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@keveleigh
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@david-c-kline
Copy link

per @keveleigh, the alloc only occurs in the editor. this pattern should be utilized going forward (v3).

given 2018 does not contain the TryGetComponent API, it feels like a more risky change for the v2 code base

@keveleigh keveleigh changed the base branch from main to prerelease/2.8.0 May 16, 2022 16:05
@keveleigh
Copy link
Contributor Author

/azp run

@keveleigh keveleigh enabled auto-merge (squash) May 16, 2022 16:05
@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@keveleigh
Copy link
Contributor Author

per @keveleigh Kurtis Eveleigh FTE, the alloc only occurs in the editor. this pattern should be utilized going forward (v3).

given 2018 does not contain the TryGetComponent API, it feels like a more risky change for the v2 code base

With the goal of trying to improve performance in as many code paths as possible (especially with regards to pointers and focus!), I think we should reevaluate taking this change for 2.8. I've retargeted the branch to ensure our tests continue to succeed there.

@keveleigh
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@keveleigh keveleigh modified the milestones: MRTK 2.x future, MRTK 2.8 May 17, 2022
@keveleigh keveleigh merged commit 28bccb2 into microsoft:prerelease/2.8.0 May 19, 2022
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.

2 participants