Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Interpolated string optimization #207

Merged
merged 7 commits into from
Jul 27, 2023

Conversation

AArnott
Copy link
Member

@AArnott AArnott commented Jul 17, 2023

Optimizes methods on the Requires, Assumes, Report and Verify classes that take string formatting args to include an overload that has special handling for interpolated strings to avoid any string formatting or allocations when the condition will not lead to a thrown exception.

This will automatically make the following code alloc-free, for example:

Requires.Argument(true, $"Some {mighty} error.");

The nice thing about the above is it scales to any number of formatting arguments and types.

Prior to this change, something like this was alloc free because we had an overload to handle 1 or 2 formatting arguments:

Requires.Argument(true, $"Some {0} error.", "mighty");

But as soon as you go beyond 2 args, or one of the args is a value-type, allocations would be required even in the success case.
But when using interpolated strings, there is never an allocation in the success case.

This optimization depends on the calling code targeting .NET 6 or better. We could theoretically make it available to .NET Framework as well, but we'd have to copy at least this large code file from .NET into this library.

AArnott added 6 commits July 17, 2023 11:36
…ng messages

This optimization avoids all allocations and message formatting except in actual failure cases.
…g args is provided

This is both a perf optimization and avoids an unexpected exception being thrown if the message happens to contain formatting placeholders such as `{0}`.
@stephentoub
Copy link
Member

stephentoub commented Jul 17, 2023

This optimization depends on the calling code targeting .NET 6 or better. We could theoretically make it available to .NET Framework as well, but we'd have to copy at least this large code file from .NET into this library.

That's not actually the case. This compiles fine on .NET Framework 4.8 and does not evaluate the interpolated string due to the condition being true:

#nullable enable

using System;
using System.Runtime.CompilerServices;
using System.Text;

internal class Program
{
    static void Main() => Example.Argument(true, $"The time is {DateTime.UtcNow}", "foo");
}

class Example
{
    public static void Argument(bool condition, [InterpolatedStringHandlerArgument(nameof(condition))] ref ArgumentInterpolatedStringHandler handler, string? parameterName)
    {
        if (!condition)
        {
            throw new ArgumentException(handler._stringBuilder?.ToString() ?? string.Empty, parameterName);
        }
    }

    [InterpolatedStringHandler]
    public struct ArgumentInterpolatedStringHandler
    {
        internal readonly StringBuilder? _stringBuilder;

        public ArgumentInterpolatedStringHandler(int literalLength, int formattedCount, bool condition, out bool shouldAppend)
        {
            _stringBuilder = condition ? default : new StringBuilder();
            shouldAppend = _stringBuilder is not null;
        }

        public void AppendLiteral(string value) => _stringBuilder!.Append(value);
        public void AppendFormatted<T>(T value) => _stringBuilder!.Append(value);
    }
}

namespace System.Runtime.CompilerServices
{
    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct, AllowMultiple = false, Inherited = false)]
    public sealed class InterpolatedStringHandlerAttribute : Attribute
    {
        public InterpolatedStringHandlerAttribute() { }
    }

    [AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = false)]
    public sealed class InterpolatedStringHandlerArgumentAttribute : Attribute
    {
        public InterpolatedStringHandlerArgumentAttribute(string argument) => Arguments = new string[] { argument };
        public InterpolatedStringHandlerArgumentAttribute(params string[] arguments) => Arguments = arguments;
        public string[] Arguments { get; }
    }
}

The decompiled Main is this:

using System;

private static void Main()
{
	bool flag = true;
	bool condition = flag;
	bool shouldAppend;
	Example.ArgumentInterpolatedStringHandler handler = new Example.ArgumentInterpolatedStringHandler(12, 1, flag, out shouldAppend);
	if (shouldAppend)
	{
		handler.AppendLiteral("The time is ");
		handler.AppendFormatted(DateTime.UtcNow);
	}
	Example.Argument(condition, ref handler, "foo");
}

The mentioned DefaultInterpolatedStringHandler.cs file provides the default implementation used by the C# compiler when interpolating into a string, e.g. string s = $"The current time is {DateTime.UtcNow}";. It's not needed when you're providing your own handler.

Note that the above example is minimalist. It doesn't provide overloads of AppendFormatted that take a format or a alignment argument, and thus attempts to use a format/alignment in the format string will faill to compile. But that's not an actual limitation; you would just need to write the code / overloads to handle whatever you want to handle.

@AArnott
Copy link
Member Author

AArnott commented Jul 17, 2023

That's not actually the case.

Thanks for reviewing, @stephentoub. If it indeed is within reach, I fully expect VS would benefit from getting there. The only question in my mind is whether it's worth the effort.

On net48, I wonder about this:

public void AppendFormatted<T>(T value) => _stringBuilder!.Append(value);

Does it compile to invoking StringBuild.Append(object)? I expect that doesn't do nearly as many things as the .NET 6 behavior calling into the ref struct would do from my own code inspection.

Note that the above example is minimalist. It doesn't provide overloads of AppendFormatted that take a format or a alignment argument, and thus attempts to use a format/alignment in the format string will faill to compile. But that's not an actual limitation; you would just need to write the code / overloads to handle whatever you want to handle.

Well, that's the thing I guess that leads to needing to copy a ton of code out of .NET 6. Looking at the AppendInterpolatedStringHandler struct that I'd have to do without, there's a ton of code there. And not doing all of it would presumably risk introducing compile-time breaks for code that updates to the latest version of this library. And adding the overloads but doing anything less than AppendInterpolatedStringHandler does within those methods scares me because I worry there are contracts that must be honored.

@stephentoub
Copy link
Member

Does it compile to invoking StringBuild.Append(object)?

Yes, but only if the condition is false. My assumption is you care a lot less about cost when it's about to fail with an exception. Plus, in the same situation today you'd be calling string.Format, which is going to similarly box a value type, call ToString on it, etc.

Looking at the AppendInterpolatedStringHandler struct that I'd have to do without, there's a ton of code there.

Again, it depends how much you care about cost. Much of that code is in the name of making it as efficient as possible to format arbitrary types. If you're only doing this formatting on failure paths, though, the extra cost likely isn't a big deal, especially when compared to the cost of instantiating, throwing, and catching an exception, in which case you don't need as much code. For example, you can implement the more full-fledged AppendFormatted with something simple like this (pseudocode):

public void AppendFormatted<T>(T value, string format = null, int alignment = 0)
{
    string result = value is IFormattable ? ((IFormattable)value).ToString(format, null) : value.ToString();
    bool left = alignment < 0;
    alignment = Math.Abs(alignment);
    if (result.Length < alignment)
    {
        string padding = new string(' ', alignment - result.Length);
        result = left ? result + padding : padding + result;
    }
    _stringBuilder.Append(result);
}

and then choose to optimize it if / as needed, knowing that it's only going to be invoked when condition is false.

@AArnott AArnott enabled auto-merge July 27, 2023 19:36
@AArnott AArnott merged commit c94db72 into microsoft:main Jul 27, 2023
@AArnott AArnott deleted the interpolatedStringOptimization branch July 27, 2023 20:05
drewnoakes pushed a commit to drewnoakes/vs-validation that referenced this pull request Sep 14, 2023
…ft#207)

Bumps [StyleCop.Analyzers.Unstable](https://github.com/DotNetAnalyzers/StyleCopAnalyzers) from 1.2.0.435 to 1.2.0.507.
- [Release notes](https://github.com/DotNetAnalyzers/StyleCopAnalyzers/releases)
- [Changelog](https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/KnownChanges.md)
- [Commits](https://github.com/DotNetAnalyzers/StyleCopAnalyzers/commits)

---
updated-dependencies:
- dependency-name: StyleCop.Analyzers.Unstable
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants