FIX: Implement caching for InputControlPath display name by josepmariapujol-unity · Pull Request #2344 · Unity-Technologies/InputSystem

josepmariapujol-unity · 2026-02-12T08:40:45Z

Description

Purpose of this PR is to fix the TODO by caching per path value and only recompute when the string actually
changes.

Testing status & QA

Check that it works as before, when inputing in the InputControlPath.

Overall Product Risks

Please rate the potential complexity and halo effect from low to high for the reviewers. Note down potential risks to specific Editor branches if any.

Complexity: 1
Halo Effect: 1

Comments to reviewers

Please describe any additional information such as what to focus on, or historical info for the reviewers.

Checklist

Before review:

Changelog entry added.
- Explains the change in Changed, Fixed, Added sections.
- For API change contains an example snippet and/or migration example.
- JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
- Jira port for the next release set as "Resolved".
Tests added/changed, if applicable.
- Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
- Performance tests.
- Integration tests.
Docs for new/changed API's.
- Xmldoc cross references are set correctly.
- Added explanation how the API works.
- Usage code examples added.
- The manual is updated, if needed.

During merge:

Commit message for squash-merge is prefixed with one of the list:
- NEW: ___.
- FIX: ___.
- DOCS: ___.
- CHANGE: ___.
- RELEASE: 1.1.0-preview.3.

u-pr · 2026-02-12T08:42:01Z

PR Reviewer Guide 🔍

(Review updated until commit `8086d03`)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪ The changes are localized to a single method in one file, implementing a straightforward caching pattern to optimize string generation in the editor UI.
🏅 Score: 100 The PR effectively addresses the performance issue (GC churn) by caching the display name based on the path string. The implementation correctly handles nulls, uses efficient string comparison, and follows Unity editor coding standards.
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected
Update review

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr

u-pr · 2026-02-12T08:44:32Z

PR Code Suggestions ✨

Latest suggestions up to 8086d03
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Skip cache update when in manual edit mode The display name calculation is unnecessary when `m_PickerState.manualPathEditMode` is active, as the UI displays the raw path string in that mode. Add a check to skip the cache update when manual edit mode is enabled to avoid needless GC churn and calculations during editing. Packages/com.unity.inputsystem/InputSystem/Editor/ControlPicker/InputControlPathEditor.cs [126-131] // Cache the display name per path value and only recompute when the string actually changes. - if (!string.Equals(path, m_CachedPath, StringComparison.Ordinal)) + if (!m_PickerState.manualPathEditMode && !string.Equals(path, m_CachedPath, StringComparison.Ordinal)) { m_CachedPath = path; m_CachedDisplayName = InputControlPath.ToHumanReadableString(path); } Suggestion importance[1-10]: 6 __ Why: The suggestion properly optimizes the code by skipping an unnecessary string calculation when `manualPathEditMode` is active, as the display name is not used in that mode. This aligns with the PR's goal of reducing GC allocations, though the actual performance gain is likely minor since `DelayedTextField` limits the frequency of path updates.	Low
More suggestions

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr

Previous suggestions

Suggestions up to commit 1b38eb1

Category	Suggestion	Impact
Possible issue	Restore missing displayName variable to fix compilation The local variable `displayName` was removed, but it is implied that subsequent code in the method (not visible in the diff) depends on it. This will likely cause a compilation error. You should declare `displayName` and assign it the value from `m_CachedDisplayName` to maintain compatibility with the rest of the method. Packages/com.unity.inputsystem/InputSystem/Editor/ControlPicker/InputControlPathEditor.cs [126-131] // Cache the display name per path value and only recompute when the string actually changes. if (!string.Equals(path, m_CachedPath, StringComparison.Ordinal)) { m_CachedPath = path; m_CachedDisplayName = InputControlPath.ToHumanReadableString(path); } +var displayName = m_CachedDisplayName; Suggestion importance[1-10]: 10 __ Why: The PR removes the local variable `displayName` which was previously defined and used. Since the subsequent code in the method (which is unchanged and not fully visible in the diff) likely relies on `displayName`, removing its definition will cause a compilation error.	High

codecov-github-com · 2026-02-12T08:44:38Z

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...tem/Editor/ControlPicker/InputControlPathEditor.cs	0.00%	6 Missing ⚠️

@@           Coverage Diff            @@
##           develop    #2344   +/-   ##
========================================
  Coverage    77.95%   77.96%           
========================================
  Files          476      476           
  Lines        97453    97522   +69     
========================================
+ Hits         75971    76031   +60     
- Misses       21482    21491    +9

Flag	Coverage Δ
inputsystem_MacOS_2022.3	`5.54% <0.00%> (+<0.01%)`	⬆️
inputsystem_MacOS_2022.3_project	`75.48% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.0	`5.32% <0.00%> (-0.01%)`	⬇️
inputsystem_MacOS_6000.0_project	`77.37% <0.00%> (+<0.01%)`	⬆️
inputsystem_MacOS_6000.3	`5.32% <0.00%> (-0.01%)`	⬇️
inputsystem_MacOS_6000.3_project	`77.37% <0.00%> (+0.01%)`	⬆️
inputsystem_MacOS_6000.4	`5.32% <0.00%> (-0.01%)`	⬇️
inputsystem_MacOS_6000.4_project	`77.37% <0.00%> (+<0.01%)`	⬆️
inputsystem_MacOS_6000.5	`5.32% <0.00%> (-0.01%)`	⬇️
inputsystem_MacOS_6000.5_project	`77.38% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_2022.3	`5.54% <0.00%> (+<0.01%)`	⬆️
inputsystem_Ubuntu_2022.3_project	`75.28% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.0	`5.32% <0.00%> (-0.01%)`	⬇️
inputsystem_Ubuntu_6000.0_project	`77.17% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.3	`5.32% <0.00%> (-0.01%)`	⬇️
inputsystem_Ubuntu_6000.3_project	`77.18% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.4	`5.33% <0.00%> (-0.01%)`	⬇️
inputsystem_Ubuntu_6000.4_project	`77.18% <0.00%> (+0.01%)`	⬆️
inputsystem_Ubuntu_6000.5	`5.33% <0.00%> (-0.01%)`	⬇️
inputsystem_Ubuntu_6000.5_project	`77.18% <0.00%> (+0.01%)`	⬆️
inputsystem_Windows_2022.3	`5.54% <0.00%> (+<0.01%)`	⬆️
inputsystem_Windows_2022.3_project	`75.61% <0.00%> (+0.01%)`	⬆️
inputsystem_Windows_6000.0	`5.32% <0.00%> (-0.01%)`	⬇️
inputsystem_Windows_6000.0_project	`77.50% <0.00%> (+<0.01%)`	⬆️
inputsystem_Windows_6000.3	`5.32% <0.00%> (-0.01%)`	⬇️
inputsystem_Windows_6000.3_project	`77.50% <0.00%> (+<0.01%)`	⬆️
inputsystem_Windows_6000.4	`5.32% <0.00%> (-0.01%)`	⬇️
inputsystem_Windows_6000.4_project	`77.50% <0.00%> (+0.01%)`	⬆️
inputsystem_Windows_6000.5	`5.32% <0.00%> (-0.01%)`	⬇️
inputsystem_Windows_6000.5_project	`77.50% <0.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...tem/Editor/ControlPicker/InputControlPathEditor.cs	`0.00% <0.00%> (ø)`

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

u-pr · 2026-02-12T12:47:37Z

Persistent review updated to latest commit 8086d03

K-Tone · 2026-02-16T11:11:16Z

Packages/com.unity.inputsystem/InputSystem/Editor/ControlPicker/InputControlPathEditor.cs

-            ////TODO: this should be cached; generates needless GC churn
-            var displayName = InputControlPath.ToHumanReadableString(path);
+            // Cache the display name per path value and only recompute when the string actually changes.
+            if (!string.Equals(path, m_CachedPath, StringComparison.Ordinal))


While technically correct, I wonder if we should use InvariantCulture to match what other string comparisons in our package do. I remember there were incoming issues with Turkish about a year ago and we moved to InvariantCulture...

Moved = we didn't specify culture rules at all, so strings would be compared differently based on the current locale

Fixing todo, caching to reduce GC in InputControlPath

1b38eb1

josepmariapujol-unity self-assigned this Feb 12, 2026

josepmariapujol-unity requested review from K-Tone and Pauliusd01 February 12, 2026 08:41

josepmariapujol-unity marked this pull request as draft February 12, 2026 10:28

Fix display name in dropdown button for control picker

8086d03

josepmariapujol-unity marked this pull request as ready for review February 12, 2026 12:46

K-Tone requested changes Feb 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

FIX: Implement caching for InputControlPath display name#2344

FIX: Implement caching for InputControlPath display name#2344
josepmariapujol-unity wants to merge 2 commits intodevelopfrom
input/fixing-cache-path-input-control-path-editor-todo

josepmariapujol-unity commented Feb 12, 2026 •

edited

Loading

Uh oh!

u-pr bot commented Feb 12, 2026 •

edited

Loading

Uh oh!

u-pr bot commented Feb 12, 2026 •

edited

Loading

Uh oh!

codecov-github-com bot commented Feb 12, 2026 •

edited

Loading

Uh oh!

u-pr bot commented Feb 12, 2026

Uh oh!

K-Tone Feb 16, 2026

Uh oh!

K-Tone Feb 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

josepmariapujol-unity commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing status & QA

Overall Product Risks

Comments to reviewers

Checklist

Uh oh!

u-pr bot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit 8086d03)

Uh oh!

u-pr bot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

codecov-github-com bot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

u-pr bot commented Feb 12, 2026

Uh oh!

K-Tone Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

K-Tone Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

josepmariapujol-unity commented Feb 12, 2026 •

edited

Loading

u-pr bot commented Feb 12, 2026 •

edited

Loading

(Review updated until commit `8086d03`)

u-pr bot commented Feb 12, 2026 •

edited

Loading

codecov-github-com bot commented Feb 12, 2026 •

edited

Loading