-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Boost performance of localized, non-static or incremental string formatting. #50330
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
It occurs to me that this is very similar to runtime/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatter.cs Line 15 in 7795971
|
@davidfowl I think you could create a new constructor for StringFormatter that understands the named hole syntax and produces the same internal representation that drives the efficient formatting. |
@davidfowl The prototype implementation has been updated to include support for the template-based model (a.k.a. named holes) model. It's just a different constructor for the CompositeFormat type, and the formatting operations then work the same as before. The new constructor returns an ordered set of template names. And unlike the semantics of LoggerMessage.Define, this version correctly handles having the same template repeated in the format string. I believe integrating this functionality into the guts of |
I concur, there's also overlap with #14484. @stephentoub the one thing not addressed by the improved string interpolation proposal is strings loaded at runtime https://github.com/dotnet/aspnetcore/blob/1870441efdc00a6c253c2a5c54c20a313fb56ee2/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs#L509-L552 Can we include this issue and #14484 into that epic? It seems like some of the assets from the string interpolation epic could be used as part of this. |
I'd be ok seeing something along those lines incorporated as an implementation detail into LoggerMessage.Define. I'm not excited right now at the idea of shipping such a CompositeFormat type publicly (in part for the same reasons we've been hesitant to expose generic overloads for string.Format itself, namely code size from generic instantiations... that ship has already sailed for LoggerMessage); even if we did, I expect you'd want any such optimization to be supported down level from .NET 6, and I don't think we'd ship such a type in its own OOB. If you did want to only target .NET 6, you could use InterpolatedStringBuilder as a building block if desired to actually build up the resulting string from the constituent components. (As an aside, #14484 isn't really an epic... it was a proposal for a specific set of APIs, which we largely declined, but the issue has stayed open as a discussion ground for alternatives.) |
That sounds reasonable but it feels like we're saying we don't need to improve the performance of runtime generated strings because of generic instantiation bloat. Re-reading #14484 makes me feel like we don't see any value in avoiding boxing in string.Format overloads because we're allocating a string anyways. Is that right?
I do want to start cross compiling the Extensions assemblies to target ns2.0, ns2.1, latest .NET build.
I meant the "improve interpolated strings" epic. @geeknoid how do you feel about burying this logic into the LoggerMessage code? We can potentially remove a bunch of it in .NET 6 by using built ins. |
With string.Format, there are other costs involved, with few benefits to such a change other than avoiding the boxing, and so you're trading a potentially huge growth in generic instantiations (every different combination of T1, T2, T3 where at least one is a value type getting its own copy) for avoiding just one of the costs involved, and a cost that can be dwarfed by the typically larger string allocation happening. It's one of the reasons I've pushed for the interpolated strings improvements the way we're doing it:
It does have the limitation that it can't be applied dynamically to format strings at runtime, and I would like to address that at some point, but I'd like to do so:
In the meantime, the two main places I see devs loading the same resource string to be processed many times in an app are:
I'm not particularly concerned with reducing string formatting costs for the latter. I would like to for the former, and we can take a big swing at that now by incorporating such a preprocessing approach into LoggerMessage.Define under the covers. It already limits what types can be used (things that can be generics), it already has the generics explosion potential and so isn't introducing a new issue, its primary purpose is preprocessing, it's what ASP.NET uses, it focuses on a known problem space, etc., plus it won't encourage use of this for things like exception messages where the tradeoffs are likely to be wrong for the end developer who may not realize it. We can get experience from that and C# 10 interpolated strings, and see if/when/how more is desired in the future. In the meantime, I'd encourage Martin to release a nuget package with his CompositeFormat, so that anyone can use it, vote with their feet, and drive further learnings for future potential incorporation. (That said, @jkotas had provided feedback on string.Format and boxing in that issue. I don't want to put words in his mouth, so he can comment here as well if he disagrees with my stance.) |
Searching through our code base, it's clear that the vast majority of uses of String.Format is around error handling. In fact, for our (service code) these strings aren't localized or customized and hence could be converted to string interpolation (someone should write a code fixer to automate converting code over). However, there are also several cases where the format string comes from either a resource file or config. For example, we format some URLs and file paths that way. And these things are done in our request paths, not in error paths. I suspect that many user-facing code is in the same boat. I suspect that framework code is not representative of application-level code in this respect, application-level code will tend to format localized content as part of main business logic. @stephentoub The use of generics in the Format API could be removed/reduced which would trigger a teeny more boxing, but would still retain most of the value that CompositeFormat provides. It factors out parsing of the format string so that the work is not done on every use. My gut says that no matter what syntax the compiler may or may not provide, in order to handle dynamic string formatting, you will end up with something like CompositeFormat to separate parsing the input string from the format operation. Anyway, I would be happy to simplify the API to reduce generic explosion in order to get the overall benefits of this model. @davidfowl As I mentioned to you this weekend, it took a few minutes to create a prototype of LoggerMessage.Define which used this new string formatting code in its implementation. It was pretty straightforward and did improve performance. If we can't get CompositeFormat in as a first class concept in the framework, I'd be totally fine to exploit it as an implementation detail of the logging system. |
It's also worth considering StringBuilder.AppendFormat. With the extensions I've integrated into my code, AppendFormat can execute without any allocations in the happy path. Formatting occurs in an on-stack span which is then copied directly into the StringBuilder. Using interpolation there would produce a transient string (unless there's some new voodoo magic coming to address this in C# 10). So barring new language features, my extensions to AppendFormat are likely the fastest way to format text into a string builder. |
It won't.
There is.
Great. Let's start with that. Thanks! |
@stephentoub Will the new string interpolation functionality support formatting straight into a span? If so, how will that look syntax-wise? |
Yes. We still need to decide on the method name (I currently have it implemented as "TryWrite"), but the syntax will look like: bool success = span.TryWrite($"{a} = {b}", out int charsWritten); or bool success = span.TryWrite(provider, $"{a} = {b}", out int charsWritten); |
@stephentoub @davidfowl There is something I don't understand about generics explosion vs. string interpolation. The cool new string interpolation model involves taking every interpolated string and expanding that inline into a series of function calls to create a span, copy literal chunks into it, format variables into it, and produce an output string. In other words, there is a custom span of IL and native code emitted for each individual interpolated string, regardless of the type of arguments it contains. If I have a Format<T0, T1, T2> generic function, it can lead to the underlying IL being converted to native code many times, depending on the specific types of the generic arguments. All reference types are folded together, and multiple uses of the function which share the same generic types in the same order will end up using the same native code. It appears to me that if the implementation of the Format<T0, T1, T2> function is small and quickly transitions to non-generic code (which my code does), or generic code with a lower arity, it could lead to considerably smaller IL and smaller native code too in the end as compared to equivalent C# 10- style string interpolation. My point being that rejecting a format function that has 3 generics on the grounds that it may lead to generics explosion seems incongruous with the introduction of new-style string interpolation which will likely have a total process-level cost in IL and native code higher than it would an equivalent number of generic format calls would cost. In other words, if the platform can bare string interpolation, it can bare format calls with generics. Am I missing something? |
(I have other things I need to focus on today. I'll respond later today or tomorrow...) |
To be clear, when I was commenting on generic specialization explosion, it was in the context of being asked about string.Format overloads that take generics, e.g. There are multiple concerns related to this, though. One concern is the raw asm size you end up with, which can be a problem for AOT. That asm also impacts JIT time for non-AOT or hybrid, and also working set (also, yes, if all of the generics in a given use are reference types than the asm will be shared, but there’s still overhead per combination in order to support that sharing). Working set in and of itself is a concern, of course, and that’s not just specific to IL/asm: any helper data structures we create and root in the assistance of a particular call site is effectively working set tied to that one call site. In this regard, I looked through LoggerMessage.Define usage in ASP.NET, and SR resource usage in Corelib. For the former, almost every log was consumed in one and only one use site, and for SR the vast majority were similarly used in only one place (a decent number were used in two, maybe three, and then there was a tail of a few that were used in many places). Point being that if you then need to declare a field and instantiate an object (which stores a lot more state under the covers) per call site, you’re effectively increasing the working set associated with that call site to include all of that stuff. Generic instantiations are just a piece of that. Let’s look at string.Format as it’s defined today. It’s not generic, so there’s only one stamp of the asm, and so the working set overhead for a particular call site is the IL/asm for that call site. Let’s use this as an example: public string StringFormat(int i, double d, Guid g)
{
return string.Format("{0} {1} {2}", i, d, g);
} The IL for that looks like:
and on .NET 5 the asm looks like:
so ~28 bytes of IL and ~177 bytes of asm. But that of course has all the deficiencies we’ve talked about:
You asked about C# 10 interpolation. The compiler generated code for the same example as above would look like: var b = InterpolatedStringBuilder.Create(2, 3);
b.AppendFormatted(i);
b.AppendLiteral(" ");
b.AppendFormatted(d);
b.AppendLiteral(" ");
b.AppendFormatted(g);
return b.ToStringAndClear(); which results in the IL:
and the asm:
So ~63 bytes of IL and ~173 bytes of asm. IL went up by ~40 bytes of IL, but asm was effectively the same, and that addresses all the deficiencies we’ve talked about:
Now let’s look at CompositeFormat. I grabbed the latest from your repo this morning. Its version of this same example would look like: private static readonly CompositeFormat s_cf = new CompositeFormat("{0} {1} {2}");
…
return _cf.Format(i, d, g); From an IL perspective, as long as you don’t go beyond the three-argument limit, as one would expect from the signature, the IL is the smallest at 15 bytes:
but the asm ends up being a different story. The call site itself here ends up being
for ~200 bytes of asm. But in addition to that, there are a few routines (Fmt and CoreFmt) that are generic on all of the parameter types, and thus are stamped for every permutation that’s used. That doesn’t necessarily equate to a stamp for each call site, but it can devolve into that:
for ~882 bytes of asm per permutation used. In addition to the IL/asm, there’s the managed object overhead associated with the given CompositeFormat, which would generally be per call site, and that’s another 3 objects / 138 bytes in this case. The above is also only when you use up to the three-argument generic version: after that, it falls back to a params array, where you no longer have the generic issue, but the only thing you gain over string.Format is the parsing at first use rather than at every use. So, my concern with pushing such a thing right now is that:
combined with us shipping something shiny new and folks potentially using it in cases they shouldn’t, e.g. paying those costs for exception resource strings. My position is not “we will never do this”, as I agree there’s merit (as I’ve stated), I’d just like to take some time with it. For example, in a not-terribly-far-off future C#/.NET we might introduce a ref struct generic constraint, which might enable ReadOnlySpan to be used, and I’d want to make sure the design played in well with that. I'd also want think about if/how it might be done without needing to manage separate type/field, e.g. could a string.Format overload be provided that was told "it's ok to cache this" in addition to being appropriately generic, and use a concurrent dictionary to map the string as a key to the parsed data. It’s also why I think we should start by incorporating it into LoggerMessage.Define: it already has all the cited issues, and isn’t a new API that represents yet another way to do something that’s already possible, rather this ends up just being an implementation detail beneath an API that is already one of the primary reasons you'd want this. And in particular, LoggerMessage.Define<T1, T2, T3> is already contributing something like ~10K of asm and managed objects for each define, so someone using it has already signed up for those overheads. |
If today |
No, and we wouldn't want that to happen. |
In the case how will we have to modify existing code with strong typed resources? Could you please show examples? |
You can keep doing exactly what you're doing today if you're fine with the perf. If your goal is to reduce the overheads associated with the formatting, then you want to create and cache the CompositeFormat from the resource, e.g. instead of: string s = string.Format(SR.WhateverResourceString, arg1, arg2); you'd do: private static readonly CompositeFormat s_whateverResourceString = CompositeFormat.Parse(SR.WhateverResourceString);
...
string s = string.Format(s_whateverResourceString, arg1, arg2); |
Thanks! My question was just about why the |
It does do parsing internally. Which takes time. Why parse over and over again? If the code isn't on a hot path, it doesn't matter. If the code is on a hot path, better to do all the preprocessing once and then reuse the fruits of your labor over and over. |
In other words, would you like to avoid an internal cache in string.Format? |
Any such cache would both cache too much (we don't need to be caching the parsing of strings destined for exceptions) and too little (it would need a cap to avoid growing unbounded), it would require synchronization which is either or both a scalability bottleneck and a performance hit, it lacks the ability for a developer to control if / how that caching happens, etc. And on top of that it would require the costs of the cache lookup. |
It seems like it should be possible to augment the resx format and associated tool such that you could mark a string resource as being a format string, and then the generated Resources.Designer.cs file would expose that string as a CompositeFormat instance instead of a raw string. Just a small convenience... |
For the codepath that is all MSBuild, we could certainly do this--we'd have to consult with the resource-designer folks in VS to make sure the additional information would be accessible if you used the UI to add or edit a string. There's an older experience where VS itself generates the C# (and generally it gets checked in). That is more commonly used today, and it uses .NET Framework resx handling code that wouldn't get the new feature. But that's not a blocker IMO. |
+💯💯 These internal caches are never great, for all the reasons mentioned 😄 |
The motivation states that this is an API for localized strings, too:
This explains why the question about caching arises. Resource manager returns a string depending on the current UI language. This means we need to remember the result of parsing that string and reset the cache if the language changes. Doing this manually for each string is too tedious. This caching should be done either in the Resource manager or in an additional intermediate layer, if not in the CompositeFormat. This intermediate layer can be generated for strong typed resources for example. Here is an example from PowerShell repository. Current code for strong typed resource string: internal static string Reason_NeverRun {
get {
return ResourceManager.GetString("Reason_NeverRun", resourceCulture);
} now would be: private static CompositeFormat s_Reason_NeverRun;
internal static string Reason_NeverRun {
get {
if (languageChanged)
{
s_Reason_NeverRun = CompositeFormat.Parse(ResourceManager.GetString("Reason_NeverRun", resourceCulture));
}
return s_Reason_NeverRun;
} If we reread the same
it says that the resource string is unknown at compile time. But I would speculate that for most applications this is not the case. I can't even think of an application that uses resource strings that are not known in advance to the developer and use an unknown number of arguments. If we dismiss this as an extreme case, we can claim that all resource strings are known at compile time and have a strictly defined number of parameters. (I'm thinking about user messages, UI elements, and how strong typed resources work.) This means that resource strings can be parsed at compile time and even converted to code. At a minimum, the developer has to create a resource file for the default language, which could act as a schema or definition of interfaces for resource strings of other languages. The source string for the default language is defacto a unique identifier (if it changes, the corresponding resource strings for other languages must be updated too). |
In a web application, |
This confirms that caching is needed in real applications. And it is even more efficient to compile resource strings directly into code - then the web application will simply call the delegate corresponding to the value of the Accept-Language header field. |
No one is saying otherwise; everyone here has stated that caching is required for this to make sense. What I objected to was caching as part of string.Format, StringBuilder.AppendFormat, etc. The consumer is responsible for caching these CompositeFormat instances in whatever manner makes sense. That could be the direct consumer, that could be tooling-generated code, etc. This provides a primitive that puts the caller in control of the cacheable item and the APIs that seamlessly consume it. |
@iSazonov, in your design, would it still be possible to keep the (code generated from) localized resources in satellite assemblies and deploy them from optional language packs? Or would you rather place the strings of all languages in the main assembly? Common Lisp has prior art for generating code from a format string at compile time (CLHS: Section 22.2.1.3 Compiling Format Strings). The type system is different there, though. |
@stephentoub I'm not trying to disprove you. I'm trying to figure out how to apply this to a specific PowerShell project. And as I pointed out in a previous post, we could modify our ResGen tool so as to get the benefits of CompositeFormat. No doubt it would work.
@KalleOlaviNiemitalo I don't see any obstacles. If we now have a ResourceManager that allows us to load and use satellite assemblies, the new manager could do the same thing only by loading delegates instead of resource strings. Especially since there is now support for unloadable assemblies. |
@iSazonov perhaps the main assembly could have an abstract class with an abstract method for each parameterized localizable string, and then the satellite assembly would define a class that extends that and implements the methods. That way it wouldn't need all the delegate instances, and the parameters could have names so that arguments in C# calls could be specified by name. Unclear:
|
@KalleOlaviNiemitalo These are implementation details. I believe C# already has everything we need to implement this script and is already in use elsewhere. The compiler and msbuild support just adds to the convenience. Plugins themselves are not new to C#. |
Looks good as proposed
namespace System
{
public class String
{
public static string Format<TArg0>(IFormatProvider? provider, CompositeFormat format, TArg0 arg0);
public static string Format<TArg0, TArg1>(IFormatProvider? provider, CompositeFormat format, TArg0 arg0, TArg1 arg1);
public static string Format<TArg0, TArg1, TArg2>(IFormatProvider? provider, CompositeFormat format, TArg0 arg0, TArg1 arg1, TArg2 arg2);
public static string Format(IFormatProvider? provider, CompositeFormat format, params object?[] args);
public static string Format(IFormatProvider? provider, CompositeFormat format, ReadOnlySpan<object?> args);
}
public static class MemoryExtensions
{
public static bool TryWrite<TArg0>(this Span<char> destination, IFormatProvider? provider, CompositeFormat format, out int charsWritten, TArg0 arg);
public static bool TryWrite<TArg0, TArg1>(this Span<char> destination, IFormatProvider? provider, CompositeFormat format, out int charsWritten, TArg0 arg, TArg1 arg1);
public static bool TryWrite<TArg0, TArg1, TArg2>(this Span<char> destination, IFormatProvider? provider, CompositeFormat format, out int charsWritten, TArg0 arg, TArg1 arg1, TArg2 arg2);
public static bool TryWrite(this Span<char> destination, IFormatProvider? provider, CompositeFormat format, out int charsWritten, params object?[] args);
public static bool TryWrite(this Span<char> destination, IFormatProvider? provider, CompositeFormat format, out int charsWritten, ReadOnlySpan<object?> args);
}
}
namespace System.Text
{
public class StringBuilder
{
public static StringBuilder AppendFormat<TArg0>(IFormatProvider? provider, CompositeFormat format, TArg0 arg0);
public static StringBuilder AppendFormat<TArg0, TArg1>(IFormatProvider? provider, CompositeFormat format, TArg0 arg0, TArg1 arg1);
public static StringBuilder AppendFormat<TArg0, TArg1, TArg2>(IFormatProvider? provider, CompositeFormat format, TArg0 arg0, TArg1 arg1, TArg2 arg2);
public static StringBuilder AppendFormat(IFormatProvider? provider, CompositeFormat format, params object?[] args);
public static StringBuilder AppendFormat(IFormatProvider? provider, CompositeFormat format, ReadOnlySpan<object?> args);
}
}
namespace System.Text
{
public sealed class CompositeFormat
{
public static CompositeFormat Parse([StringSyntax(StringSyntaxAttribute.CompositeFormat)] string format);
public static bool TryParse([StringSyntax(StringSyntaxAttribute.CompositeFormat)] string format, [NotNullWhen(true)] out CompositeFormat? compositeFormat);
public string Format { get; }
}
} |
@bartonjs, @terrajobst, in the API review for this, you were asking about what ambiguities made me remove new overloads that didn't take an IFormatProvider. Here's an example: using System;
public class C
{
public static string M(string format, object[] args) => MyString.Format(null, format, args);
}
public class MyString
{
public static string Format(IFormatProvider? provider, string format, params object[] args) => null;
public static string Format<T1, T2>(CompositeFormat format, T1 arg1, T2 arg2) => null;
}
public class CompositeFormat {} That fails to compile due to the call being ambiguous: it could interpret the null as either an IFormatProvider or a CompositeFormat, the format string as either a string or a generic T1, and the object[] as either the args or a generic T2, and there's no tie-breaker in overload resolution in the language to disambiguate those. Similar issue exists for StringBuilder.AppendFormat. This seemed like enough of a deal breaker for me that I kept those overloads out, but during the meeting we did say we'd want to add them if the overloads turned out not to be problematic. Do you agree we shouldn't add them? |
EDITED by @stephentoub 1/13/2023:
Latest API proposal at #50330 (comment)
Background and Motivation
The strides in .NET around string formatting over the last few years have been significant, and .NET 6 with C# 10 promise further substantial improvements to string interpolation performance. However, string interpolation has a serious constraint: it only works when the format strings are known at compilation time.
For any scenario where the text to format is not known at compilation time, such as for localized content being pulled from a resource file, then classic
String.Format
andStringBuilder.AppendFormat
remain the workhorses.This proposal is designed to deliver substantial performance improvements to classic composite string formatting by hoisting the expensive process of parsing the composite format string to initialization time, enabling the hot path of performing string formatting to execute up to 5x faster than using classic functions.
The model is logically similar to how you can compile a regex a priori and use it repeatedly to seek matches. Here, you parse and process the format string once, and then use it to repeatedly format strings.
Please see here for a fully functional implementation, with complete test coverage and a banchmark.
Proposed API
The primary API surface proposed here is fairly minimal. The
CompositeFormat
type has a primary constructor, then various overloads forFormat
andTryFormat
.In addition to the feature set shown above, it's also desirable to add extension methods to the
StringBuilder
which leverage the same model for enhanced formatting performance in that context. See here for the prototype API for these.Usage Examples
Alternative Designs
Rather than implementing the
Format/TryFormat
methods as instance methods on theCompositeFormat
type, they could be integrated directly into theString
type as first class methods. This would provide a clean model. Similarly, the extensions methods forStringBuilder
could be baked in directly to theStringBuilder
type.Using such an approach doesn't yield additional performance, it's really just a matter of how the API is suraced.
Benchmarks
Here are benchmarks for the current prototype implementation:
Risks
None that I'm aware.
Implementation Notes
The prototype implementation I linked to above requires .NET Standard 2.1 as a baseline.
The implementation as presented depends on an internal
StringMaker
type, which is a highly efficient replacement forStringBuilder
packaged as aref readonly
struct. It would be easy to reimplement this code on top of equivalent data types being created to support the C# compiler's enhanced interpolation features in C# 10.The text was updated successfully, but these errors were encountered: