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

Avoid unnecessary string copies #4201

Merged

Conversation

chriche-ms
Copy link
Member

@chriche-ms chriche-ms commented Jun 22, 2020

Related Issue

Fixes #4198

Description

Updates various C++ functions to accept and/or return strings by reference (const-ref and r-value) to avoid unnecessary string copies. Also makes minor tweaks to the UTF-8 <-> UTF-16 string conversion code. These changes aim to improve card parsing and rendering performance.

How Verified

Ran the existing unit tests and performed manual testing by parsing and rendering a number of adaptive cards used by the Cortana in Windows app.

Microsoft Reviewers: Open in CodeFlow

@chriche-ms chriche-ms requested a review from paulcam206 June 22, 2020 11:22
@chriche-ms chriche-ms self-assigned this Jun 22, 2020
@chriche-ms chriche-ms changed the title Chriche/uwp performance stringconversion Avoid unnecessary string copies Jun 22, 2020
@paulcam206 paulcam206 requested a review from RebeccaAnne June 22, 2020 23:48
@shalinijoshi19 shalinijoshi19 added the Area-Performance Tracking issues around perf/renderer latency etc label Jun 24, 2020
@ghost ghost added the no-recent-activity label Jun 29, 2020
@ghost ghost assigned dclaux Jun 29, 2020
@ghost
Copy link

ghost commented Jun 29, 2020

Hi @chriche-ms. This non-spec pull request has had no recent activity for the past 5 days . Please take the necessary actions (review, address feedback or commit if reviewed already) to move this along.

Copy link
Member

@shalinijoshi19 shalinijoshi19 left a comment

Choose a reason for hiding this comment

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

:shipit:

@ghost ghost removed the no-recent-activity label Jun 30, 2020
@ghost
Copy link

ghost commented Jun 30, 2020

Hi @shalinijoshi19; Thanks for reviewing this previously stale pull request. Resetting staleness. @chriche-ms FYI.

@shalinijoshi19
Copy link
Member

@RebeccaAnne is added to the review. #Closed

@shalinijoshi19
Copy link
Member

@paulcam206 is added to the review. #Closed

@chriche-ms chriche-ms merged commit b170c72 into microsoft:main Jul 1, 2020
@chriche-ms chriche-ms deleted the chriche/uwp_performance_stringconversion branch July 1, 2020 10:33
@shalinijoshi19 shalinijoshi19 added this to the 20.06 milestone Jul 10, 2020
rankush pushed a commit to rankush/AdaptiveCards that referenced this pull request May 8, 2024
* Improve performance of utf8 <-> utf16 conversion code

* Small tweaks to UWP string conversion functions

* Remove unnecessary string conversions

* Remove unnecessary string copies

* Update various functions to return strings by reference

Co-authored-by: Paul Campbell <paulcam@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Performance Tracking issues around perf/renderer latency etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UWP] Unnecessary string copying
4 participants