Skip to content

Minor tweaks to avoid some allocations#514

Merged
stephentoub merged 1 commit intodotnet:masterfrom
stephentoub:fixup_concat_allocs
Feb 15, 2015
Merged

Minor tweaks to avoid some allocations#514
stephentoub merged 1 commit intodotnet:masterfrom
stephentoub:fixup_concat_allocs

Conversation

@stephentoub
Copy link
Copy Markdown
Member

Just some small tweaks to fix up some places where:

  • Char literals were used in string concatenation, resulting in boxing and string allocations that could be avoided if a string literal were used instead
  • Value types were used in string concatenation, resulting in unnecessary boxing allocations

Most of these are under the assumption that #415 will not be merged.

Fix up some places where:
- Char literals were used in string concatenation, resulting in boxing and string allocations that could be avoided if a string literal were used instead
- Value types were used in string concatenation, resulting in unnecessary boxing allocations
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

I'm really on the fence about this. While i totally understand the concern about allocations, it's really really unpleasant to have to put in this sort of microoptimization, instead of just writing clear code that the system appropriately optimizes.

Note: as for #415, i think it should be merged in, with just a small tweak. Specifically, it should only apply to the core set of special value types that C# knows are pure. i.e. bools, char, and all the numerics. This would get you the majority of savings, without having to worry about changing semantics, while still allowing code to be written cleanly.

@stephentoub
Copy link
Copy Markdown
Member Author

Some of these, e.g. the string literal instead of a char literal, are IMHO exactly as readable (and you'd want to do those even with what's in #415; I noted in that pull request it'd be nice if the compiler was allowed to do that substitution as well). Others that just add .ToString () don't seem particularly difficult or less clean. I agree in other cases it's less legible.

@gafter
Copy link
Copy Markdown
Member

gafter commented Feb 14, 2015

👍

@gafter gafter added Area-Compilers Tenet-Performance Regression in measured performance of the product from goals. labels Feb 14, 2015
@jaredpar
Copy link
Copy Markdown
Member

👍

@heejaechang
Copy link
Copy Markdown
Contributor

+1

unless we can relax performance RI gate or relax our performance goal somehow (100% typing responsiveness under 65ms) , unfortunately we need to care about these kinds of stuff in managed code.

many people has been pushing somehow relaxing performance RI gate (especially when regression is 50ms or something below like that). Not sure how type script is doing RPS test but until process changes, not much we can do.

stephentoub added a commit that referenced this pull request Feb 15, 2015
Minor tweaks to avoid some allocations
@stephentoub stephentoub merged commit 298bdf8 into dotnet:master Feb 15, 2015
@stephentoub stephentoub deleted the fixup_concat_allocs branch February 15, 2015 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Tenet-Performance Regression in measured performance of the product from goals.

6 participants