-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Add String.Format overloads to avoid unnecessary allocations #14484
Comments
Do the additional FormattableStringFactory members require Roslyn changes to be consumed with the nice syntax? |
I think the parameters in the new overloads should have exactly the same names as in the old overloads, so that if someone called |
I need to double-check, but I'm pretty sure Roslyn is already designed to make use of such overloads to |
The 0-based parameter names seem a little strange, but now that you mention keeping the naming the same as the old overloads, the format items in the format string are 0-based, so it makes sense that the parameter names match that. In that case, I'm guessing we'd want the naming of the generic type parameters to match the naming of the arguments: So instead of this: public static string Format<T>(string format, T arg);
public static string Format<T1, T2>(string format, T1 arg1, T2 arg2);
public static string Format<T1, T2, T3>(string format, T1 arg1, T2 arg2, T3 arg3); This: public static string Format<T0>(string format, T0 arg0);
public static string Format<T0, T1>(string format, T0 arg0, T1 arg1);
public static string Format<T0, T1, T2>(string format, T0 arg0, T1 arg1, T2 arg2); Or maybe even |
We did talk about the generic overloads on FormattableStringFactory as part of the review but we didn't believe we had enough runway to get the compiler to actually use those in this product release. (cc @gafter). As for the string.Format overloads we did end up doing some work this release to make the arg0/arg1/arg2 overloads actually avoid the params array allocation internally (yeah until recently we didn't actually even optimize that even though we had the overloads). While we did do that one thing we realized is that string.Format is extremely bad about allocations internally and we don't believe that even eliminating the params array allocation ended up improving performance as much as we would have liked it to. I'm primarily saying that because I have to wonder if a few boxing allocations is really going to have much of an impact when what is really needed is a much larger redoing of the underlying formatting code (which I'm not sure is even possible in a compatible way). Do you have any data about whether or not eliminating these boxing allocations will end up helping much? By the way this doesn't mean these aren't worth adding I'm just trying to collect some more data. |
The C# and VB compilers will use appropriate overloads on |
My experience is that the inner workings of string.Format (actually StringBuilder.AppendFormat) is quite efficient. The allocations are what you'd expect. Most of the time we get a hit in StringBuilderCache, so there's no char[] allocation. It would be interesting to see if one could implement this, avoiding boxing but at the same time avoiding stamping out 3 new generic versions of StringBuilder.AppendFormat. |
There's already an analyser that does this. The tricky part is keeping it up to date with the changes, for instance it currently doesn't handle the recent changes that got rid of unnecessary boxing in String.Concat. Probably the same applies for the change that removed params array allocations, that has been mentioned elsewhere in this thread. The robust way to do this would be for it to look at the IL produced, but that's going to be expensive for an Analyser (although I did give it a go and Roslyn combined with Cecil makes it nice and easy). |
@weshaggard, I have some data in the conclusion below, but first some background... Consider the following example: DateTime now = DateTime.UtcNow;
Guid id = Guid.NewGuid();
decimal value = 3.50m;
string result = string.Format("{0:s} - {1:B}: The value is: {2:C2}", now, id, value); Let's track of the allocations with the current implementation:
13 allocations! Note that there's also a These allocations can be reduced significantly!
Down to just 1 allocation for the resulting string! A) Generic overloadsAvoids the boxing allocations. Pretty straightforward. B) Low-hanging fruit (avoiding/mitigating the internal
|
Shameless plug of my own work: https://github.com/MikePopoloski/StringFormatter I too needed string formatting without allocations. It'd be nice to take advantage of some of that dedicated code built into the CLR to make it faster, but it does alright on its own as pure C#. |
@terrajobst, Immo, this is an API suggestion. Could this be added to the agenda for an API review? Even if there's some opposition to the |
Hi Justin, how does one include the above overloads. i mean defining a new class "System" throws error - no body implementation for the overload methods or they arent defined as abstract, partial, extern etc. |
I know it's been a while, but we'd love to see this over at Stack Overflow - @terrajobst can you please ping here when the API review is up next so we can listen in? |
Generally yes, I believe with have everything we need. However, we're currently focusing on getting RTM ready, which means we're focusing our reviews on exposing APIs that already exist in .NET Framework to simplify porting as well as adding new APIs that are required to unblock other components and tools. That's why it's currently in the |
We could get the maximum ApproxMaxCharLength<T>( obj : T ) : Int
{
return match( typeof( T ) ) with
{
| string -> ((string)obj).Length;
| bool -> 5; // false
| byte -> 3; // 256
| sbyte -> 4; // -128
| char -> 1; //
| uint16 -> 6; // 65535
| int16 -> 6; // -32768
| uint32 -> 11; // -2147483648
| int32 -> 10; // 4294967295
| uint64 -> 20; // 18446744073709551615
| int64 -> 20; // –9223372036854775808
| ... -> 0;
// not done the floating point ones.
}
} An initial first approximation of the string length, could be calculated thus. var intialSize = args.Sum( ApproxMaxCharLength ) + TextOfFormatString.Length; I think this would be an over allocation. Note this is excluding any variation caused by the culture and additional alignment and format specifiers on the arg hole eg At which point (not done any deep analysis) would the extra processing to get a good estimate of the size of string to allocate, outweighs the processing cost of just blindly doing the building of the string? |
@AdamSpeight2008 This issue is mainly about the allocations due to boxing. |
The APIs look fine, however, the only way we could benefit from this work is if our implementation avoids the boxing all the way down, which would be non-trivial amount of work. Also, it's questionable without a benchmark how big the savings are, because formatting will probably cause a bunch of allocations anyway. It might be better to leave these APIs as-is and instead use the new formatting APIs that @KrzysztofCwalina is working on. @KrzysztofCwalina, what are your thoughts on this? |
Remember that the benefits here are extreme because just by recompiling, you get them. That huge, free adoption should be factored into the effort costing. This helps all code, rather than just new code. I don't see Perhaps I'm missing something though - doesn't |
|
I like the improvement in-place for the reson @NickCraver spells out. I think we should add these APIs. BTW, the new formatting APIs @terrajobst mentioned are described at https://github.com/dotnet/corefxlab/wiki/System.Text.Formatting. But I think an in-place addition will be complementary. |
This should be data-driven decision. It is unclear to me whether saving extra allocations is worth it to justify the extra code in this case. String.Format will always allocate and it will always be relatively slow API. It is unclear to me whether changing it to allocate a bit less garbage at the cost of pile of extra code is going to measurable improve anything important. |
The trouble is string interpolation is too good a feature not to use, even for those of use that normally try to avoid allocations at all costs... (though normally used in lesser taken paths like exception handling) |
Agree. In the above @justinvp example, there 13 allocations. 9 of them are internal. It should be possible to eliminate all internal ones without changing the shape of the API, so that should be done first. |
Next step: Improve current memory allocations of existing APIs as much as possible -- the result will tell us if we still need new APIs. |
How would we ever eliminate the boxing here without the API additions? I agree with internal eliminations being the first step, but if we're after absolute performance, I don't see a way around the second step. |
The no-allocation formatters being developed in corefxlab are going after absolute performance.
|
(Aside regarding the original proposed API rather than the current discussion, would |
@jkotas I agree on Either way, we need to measure. The trouble is measuring this accurately means really doing it for comparison...to see if it's worth doing. Vicious circle we have here :) |
@geeknoid, what are the advantages of that over my cited proposal? From a usability perspective, I'd prefer being able to write |
How do format specifiers work in your proposal @stephentoub ? |
We'd define a pattern for Append methods, where the first argument is the thing to be formatted, if there's a second it's a format string, etc. |
I've now added support for a Span-based TryFormat version which results in no allocations. This version is over 4x as fast as String.Format and induces no garbage. After 20 years, there's finally a formatting function in .NET that can compare in perf with C's sprintf :-) Here are the benchmark results
|
@stephentoub The benefits to my proposal is that:
I think the C# compiler can be taught about this type and magically generate the static StringFormatter instances for any use of string interpolation. Thus, existing code would just suddenly get faster by recompiling. Additionally, I think there could be a Roslyn code fixer which offers to convert any use of String.Format with a static format string into an interpolated string. Those cases would also then magically benefit from improved perf. Also I think my implementation for a non-boxing version of the ParamsArray type could be used to eliminate boxing in existing APIs like String.Format, making those a touch faster too. |
@geeknoid can you share benchmark code or link it please? While this is kinda off topic but I feel that there might be a way to make faster F# printf with things you're proposing. |
@En3Tho looks like it's at https://github.com/geeknoid/FastFormatting |
Thanks.
I expect it would be smaller IL. I'm skeptical it'd be meaningfully faster for creating strings. And it would almost certainly result in more asm, in particular as every unique combination of arguments involving a value type would result in generic specialization for all of the generic pieces, and that's likely to be non-trivial; in contrast, with the builder approach, you only end up worst case with a generic-specialization per type formatted rather than per combination of types formatted.
I agree the TryFormat aspect of it is nice. With the builder approach, you can get that as well, but it involves a copy. In both approaches, it's also hard to claim all allocations are completely eliminated, as the moment you need to utilize IFormattable, you're likely incurring string allocations for each argument being formatted, only not when the instance is able to utilize a cached string. I also don't see how the proposed approach supports arbitrary numbers of arguments without allocation. At some point you need one more argument than can be supported by the explicit strongly-typed overloads provided, and you end up with the params object[] overload. One of the nice things about the builder approach is it supports any number of arguments strongly-typed, as each argument is formatted independently with a single Append call.
This also hints at one of its biggest flaws IMO, but maybe you have a good solution for this. I think anything new we do in this space must support formatting
With the existing overloads? I don't see how, since the boxing is inherent to the existing APIs. It's also inherent to the existing implementation, which relies heavily on being able to pass the arguments around as object. If you're suggesting new generic string.Format overloads like those proposed in the very first post, that would also require plumbing that through the whole formatting implementation; I'd be interested in understanding what that implementation would look like, and similar to my previous comments, what impact that would have on bloating asm size in a typical apps (especially for AOT compilations, Blazor, Xamarin apps, etc., where presumably existing usage would start binding to the new more-strongly-typed overloads). |
Yes, you've found all the flaws of the current implementation :-). Some more thoughts:
I can't think of a way to support ReadOnlySpan in my current design, just like it can't be supported by any of the existing formatting code in .NET. That doesn't make me too sad though. The solution there is to fix the GC to allow Span in the heap :-) |
@stephentoub So what I've done in my stuff is three things:
Exposing the extended ValueStringBuilder as a public type would be sufficient to implement the builder pattern you describe. String interpolation would immediately benefit. This could support all the normal formatting features (width, justification, etc) Exposing ValueStringBuilder would make it possible for developers to implement 0-alloc string concatenation by supplying a span to the constructor of ValueStringBuilder. It's a bit clumsy, but it'll be quite fast. To let developers do 0 alloc string formatting, then you can add the StringFormatter type with its TryFormat method. Or, you could get fancy and enhance string interpolation with new syntax such as:
This would let the C# compiler pass the span to the ValueStringBuilder constructor and voila, slick no-alloc string formatting. |
That syntax is too vague in this case. Should it return bool and chars written, should it only return chars written and -1 in bad case, or should it completely hide details and just suddenly blow up with our of memory if anything goes wrong? I think that sticking to Method calls is better here. |
@stephentoub I've updated the design at https://github.com/geeknoid/FastFormatting. From the README:
This new code maintains the same perf characteristics as I had previously. The code has nearly 100% code coverage and compatibility with classic String.Format semantics is explicitly tested for. I think How's that sound? |
Latest perf tests. These are all trying to format the same set of inputs and produce the same final string:
|
Isn't StringMaker essentially just ValueStringBuilder, and basically the same builder approach I was proposing? The only thing that differs is I was suggesting a pattern in addition, but that's just to enable additional scenarios, namely an API that wants to take a particular builder so as to a) have raw access to its underlying buffer and b) have control over the Append methods exposed so as to have more control over the formatting. But we'd have a built-in implementation of it that the compiler could use when targeting string, such that existing string interpolation just go more efficient in its implementation without semantic changes. So, I'm not clear on fundamentally what's different here? (Note that if we do go with such an approach, such a type would likely live in the compiler services namespace, and would be discouraged for general direct use: in general we're wary of exposing structs that wrap pooled resources, even if ref structs, that would make it easy to accidentally copy them and dispose multiple times, double-freeing a buffer.)
As noted above, this seems like it's just ValueStringBuilder. If from an implementation perspective there are things you've done in your implementation that you've found to be more efficient, please feel free to submit a PR to improve ValueStringBuilder's implementation / reduce its overheads.
I think you're now proposing two separate things:
Am I understanding correctly? |
ps I'm on a stay-cation now and will be slow to respond until the new year. Thanks, and happy holidays. |
@stephentoub Yes, that is correct. A layered model with a builder pattern and a composite format string formatter built on top. Indeed, StringMaker exposes the builder pattern that you suggested. I realized that my original StringFormatter could easily be cleaved into two parts. One being a builder, and the other being the preprocessed string formatting functionality. And as you can see from the benchmarks, you were totally right that the builder pattern is faster at runtime. Makes sense, since StringFormatter is extra crap on top of the builder and is thus slower. StringMaker is based on the same principle as ValueStringBuilder, but it is different in a few ways:
I think as it is, the StringMaker type can handle random types efficiently:
In the common case, the C# compiler should be able to use the 0-alloc variant of StirngMaker and deliver a near optimal formatting experience. The only thing better would be if the compiler support syntax to do interpolation into an existing span. |
Sorry for the delay, @geeknoid.
Yes, all of those are part of productizing a ValueStringBuilder-like type for the purposes of a C#-based string interpolation target. We've been saying "ValueStringBuilder" just because it's the general approximation of what's needed. There are other aspects of it that would differ as well; as noted, for example, we would likely want to hide it away in System.Runtime.CompilerServices and with some less appealing name to highlight that it's a relatively dangerous type to use directly (as is any struct that rents/returns arrays from a global pool that others use). I spoke about all of this with @jaredpar last week, and it's definitely something he's interested in pushing ahead, but I don't know if it'll make C# 10 or not. For me, this is really about the C# language feature being made more efficient rather than it being about a type a developer can use directly, though if someone wanted to directly use something from System.Runtime.CompilerServices, of course they could (we would need to decide whether to support the TryFormat approach even if it wasn't going to be used by the compiler). I think the 90% case here is just enabling: string result = $"A={a} B={b}"; to efficiently generate that string. This does bump up against something we'll need to solve, though, which is how does the compiler get the stack space to use to seed the ValueStringBuilder, or is it forced to always use a rented buffer. In other words, it'd be nice if the compiler could generate for the above something like: var vsb = new ValueStringBuilder(stackalloc char[SomethingReasonable]);
vsb.Append("A=");
vsb.Append(a);
vsb.Append(" B=");
vsb.Append(b);
string result = vsb.ToString(); but it's not clear at present exactly what it should do for ValueStringBuilder vsb = default;
vsb.Append("A=");
vsb.Append(a);
vsb.Append(" B=");
vsb.Append(b);
string result = vsb.ToString(); |
@stephentoub thanks for the feedback. Improving the interpolated string perf is key, but I think strictly focusing on that use case is missing other significant opportunities for perf gains. As I mentioned above, there are two other key scenarios I'm enabling here:
I'm content to deploy these technologies within our broad team and reap the benefits, but I would rather these improvements be rolled into .NET proper so that everyone can benefit. Just point me in the right direction... |
Until something else happens, you can of course ship a nuget package that anyone can use :) |
For the interpolated string case I feel very good about the C# 10 chances assuming that runtime has the bandwidth to get the
This is the only part that makes me hesitate a bit. If we feel like interpolated strings can be solved independently of other string formatting issues then I'd actually hoped to move forward rather soon. If we think they're related though then we'll need to push forward with the other design points before the C# work.
This is the starting point I would prefer. There are several types which would benefit from the use case of "pass me a pre-allocated Span I can use as an initial buffer". One other case was a Alternatively if it helps the compiler could also pass some additional info to the constructor. For example it can give the length of the constant part of the string plus the number of arguments if that would be beneficial to |
FWIW, I do like the additional support of being able to a) TryFormat the result of string interpolation rather than always going into a string, and b) being able to seed the builder with the destination output buffer so that there's no additional copying needed. For (a), I could imagine us implementing that for string interpolation where the compiler transforms something like: bool success = $"A={a} B={b}".TryFormat(span, out int charsWritten); into code like: var vsb = new ValueStringBuilder();
vsb.Append("A=");
vsb.Append(a);
vsb.Append(" B=");
vsb.Append(b);
bool success = vsb.TryFormat(span, out int charsWritten); And for (b), I could imagine us implementing that for string interpolation where the compiler transformed that same example into something along the lines of: charsWritten = 0;
var vsb = new ValueStringBuilder(span, ref charsWritten);
bool success =
vsb.TryFormat("A=") &&
vsb.TryFormat(a) &&
vsb.TryFormat(" B=") &&
vsb.TryFormat(b) &&
vsb.TryComplete(); |
@jaredpar As written, you can use my StringMaker type (a.k.a. ValueStringBuilder per your name) independent of the other layers. The StringFormatter type I've got defined is an independent thing that uses StringMaker in its implementation. So it's easy to peal off StringMaker and use that directly. |
Just to circle back, @333fred has a spec that we're moving forward with that enables something very similar to what I outlined in #14484 (comment) and #14484 (comment). He can share the details when he's ready, but I think folks will be happy. |
I was just submitting a PR with the spec: dotnet/csharplang#4486. Edit: fixed link. |
The interpolated string handlers feature in .NET 6 is much more flexible, usable and performant than these proposed APIs. I'm going to close this issue, please correct me if I'm wrong. |
It's pretty common to pass value types to
String.Format
, unfortunately this results in unnecessary boxing allocations.Rationale
Consider the following:
Both the traditional call to
String.Format
and the use of the string interpolation language feature (which is just syntactic sugar forString.Format
) requires 3 boxing allocations.Proposed API
I'd be happy to contribute an implementation and tests.
The text was updated successfully, but these errors were encountered: