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

Refactor string.TryCopyTo #111314

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Jan 12, 2025

Total bytes of base: 40205636
Total bytes of diff: 40201837
Total bytes of delta: -3799 (-0.01 % of base)
Total relative delta: -5.52
    diff is an improvement.
    relative diff is an improvement.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 12, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jan 12, 2025
@xtqqczze
Copy link
Contributor Author

@MihuBot

@EgorBo
Copy link
Member

EgorBo commented Jan 12, 2025

It's not clear why it's better. In fact, your change assumes that the destination is more likely to be not big enough which is not true. Generally, if you think some C# massage improves codegen, please file an issue against JIT instead

@huoyaoyuan
Copy link
Member

huoyaoyuan commented Jan 12, 2025

Moreover, assembly size difference is not the correct measurement of performance. Extra instructions are sometimes required to fine tune CPU behavior, especially branch prediction. "Optimize for size" and "optimize for speed" are also different configurations for compilers, including RyuJIT and C compilers.

To demonstrate a real performance improvement, please include numbers of performance benchmarks, and/or the detailed ASM code for comparison. Generally it's not the goal to minimize the size number only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution Indicates that the PR has been added by a community member needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants