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

Possible optimization in interpolated string: inline interpolated constants #4678

Closed
gafter opened this issue Aug 20, 2015 · 5 comments
Closed
Labels
Area-Compilers Feature Request Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on. Tenet-Performance Regression in measured performance of the product from goals.

Comments

@gafter
Copy link
Member

gafter commented Aug 20, 2015

if an expression hole has a constant string value and no alignment or format, treat it as a literal instead of an expression. This would erase string.Format calls for scenarios like "throw new ArgumentException($"{nameof(argument)} should be something else".

Migrated from TFS/DevDiv 1097932

@gafter gafter added Area-Compilers Tenet-Performance Regression in measured performance of the product from goals. labels Aug 20, 2015
@gafter gafter added this to the 2.0 milestone Aug 20, 2015
@AdamSpeight2008
Copy link
Contributor

That should optimise down to just a string constant, not a Interpolation String.
NameOf( argument )
What about the case of multiple expression holes, one constant one not?
You can also extend it to any constant, not just strings. as well as including , alignment. As it is just padding the string with spaces. Since we are using constants, it could be worked out a compile-time.

Should this be more of an Analyser and Code-Fix? (Eg String-Format Diagnostics. Doesn't offer code-fixes yet))

@khellang
Copy link
Member

khellang commented Nov 5, 2015

@gafter Do you know if there's an existing issue on optimizing interpolated strings where you're basically just doing concatenation? (I couldn't find one)

So that var result = $"{a}{b}"; becomes var result = string.Concat(a, b); instead of var result = string.Format("{0}{1}", a, b);, at least for a certain amount of parameters.

See aspnet/HttpAbstractions#460 (comment) for reference.

@benaadams
Copy link
Member

@khellang @gafter Additionally automatically calling .ToString() on valuetype params; currently:

public static string InterpolatedBox(int i)
{
    return $"? {i}";
}

boxes as

.method public hidebysig static string  InterpolatedBox(int32 i) cil managed
{
  // Code size       17 (0x11)
  .maxstack  8
  IL_0000:  ldstr      "\? {0}"
  IL_0005:  ldarg.0
  IL_0006:  box        [System.Runtime]System.Int32
  IL_000b:  call       string [System.Runtime]System.String::Format(string,
                                                                    object)
  IL_0010:  ret
} // end of method

vs adding ToString like

public static string InterpolatedToString(int i)
{
    return $"? {i.ToString()}";
}

which doesn't box

.method public hidebysig static string  InterpolatedToString(int32 i) cil managed
{
  // Code size       18 (0x12)
  .maxstack  8
  IL_0000:  ldstr      "\? {0}"
  IL_0005:  ldarga.s   i
  IL_0007:  call       instance string [System.Runtime]System.Int32::ToString()
  IL_000c:  call       string [System.Runtime]System.String::Format(string,
                                                                    object)
  IL_0011:  ret
} // end of method

As its known at compile time what is happening

@gafter
Copy link
Member Author

gafter commented Nov 5, 2015

@khellang I don't know if there is an issue for it.

@gafter
Copy link
Member Author

gafter commented Nov 12, 2015

See #6738 (comment) for why this is likely to never happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Feature Request Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on. Tenet-Performance Regression in measured performance of the product from goals.
Projects
None yet
Development

No branches or pull requests

4 participants