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

Add System.Text.CompositeFormat #80753

Merged
merged 3 commits into from Jan 20, 2023
Merged

Conversation

stephentoub
Copy link
Member

Fixes #50330

@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost assigned stephentoub Jan 17, 2023
@stephentoub
Copy link
Member Author

stephentoub commented Jan 17, 2023

Benchmark via the higher-level LoggerMessage which this PR also updates to use this:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using Microsoft.Extensions.Logging;
using System;

[MemoryDiagnoser]
public partial class Program
{
    static void Main(string[] args) => BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);

    private Action<ILogger, int, Exception> _message = LoggerMessage.Define<int>(LogLevel.Critical, 1, "The value is {0}.");
    private ILogger _logger = new MyLogger();

    [Benchmark]
    public void Format() => _message(_logger, 42, null);

    sealed class MyLogger : ILogger
    {
        public IDisposable BeginScope<TState>(TState state) => null;
        public bool IsEnabled(LogLevel logLevel) => true;
        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter) => formatter(state, exception);
    }
}
Method Toolchain Mean Error StdDev Ratio Allocated Alloc Ratio
Format \main\corerun.exe 105.22 ns 2.035 ns 2.261 ns 1.00 144 B 1.00
Format \pr\corerun.exe 83.60 ns 0.959 ns 0.850 ns 0.79 80 B 0.56

@ghost
Copy link

ghost commented Jan 17, 2023

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #50330

Author: stephentoub
Assignees: stephentoub
Labels:

area-System.Runtime, new-api-needs-documentation

Milestone: -

@tarekgh
Copy link
Member

tarekgh commented Jan 18, 2023

https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-80753-merge-28a7bc7e27914ca7aa/System.Runtime.Tests/1/console.dade11dc.log?helixlogtype=result

System.InvalidOperationException : The DebuggerDisplayAttribute for System.Text.CompositeFormat contains the expression "Format".
   at System.Diagnostics.DebuggerAttributes.ValidateDebuggerDisplayReferences(Object obj) + 0x3e4
   at System.Text.Tests.CompositeFormatTests.DebuggerDisplay_ShowsFormat() + 0x5c
   at System.Runtime!<BaseAddress>+0x1166c2c
   at System.Reflection.DynamicInvokeInfo.Invoke(Object, IntPtr, Object[], BinderBundle, Boolean) + 0xd8

@stephentoub
Copy link
Member Author

/benchmark

@stephentoub
Copy link
Member Author

/benchmark test test runtime

@stephentoub
Copy link
Member Author

/benchmark help

@pr-benchmarks
Copy link

pr-benchmarks bot commented Jan 19, 2023

Crank Pull Request Bot

/benchmark <benchmark[,...]> <profile[,...]> <component,[...]> <arguments>

Benchmarks:

  • microbenchmarks: .NET Performance micro benchmarks (default filter: "LinqBenchmarks", change by adding "--variable filter=...")
  • plaintext: TechEmpower Plaintext Scenario - ASP.NET Platform implementation
  • json: TechEmpower JSON Scenario - ASP.NET Platform implementation
  • fortunes: TechEmpower Fortunes Scenario - ASP.NET Platform implementation
  • httpclient-kestrel-configured: HttpClient Benchmark (default: HTTP/1.1 GET 8K)

Profiles:

  • aspnet-perf-win: Intel/Windows 12 Cores
  • aspnet-citrine-win: Intel/Windows 28 Cores

Components:

  • runtime
  • libs

Arguments: any additional arguments to pass through to crank, e.g. --variable name=value

@stephentoub
Copy link
Member Author

/benchmark microbenchmarks --variable filter=Formatting aspnet-perf-win runtime

@stephentoub
Copy link
Member Author

/benchmark microbenchmarks aspnet-perf-win runtime --variable filter=Formatting

@pr-benchmarks
Copy link

pr-benchmarks bot commented Jan 19, 2023

Benchmark started for microbenchmarks on aspnet-perf-win with runtime and arguments --variable filter=Formatting. Logs: link

@pr-benchmarks
Copy link

pr-benchmarks bot commented Jan 19, 2023

An error occured, please check the logs

@stephentoub
Copy link
Member Author

/benchmark microbenchmarks aspnet-perf-win runtime --variable filter=Formatting

@pr-benchmarks
Copy link

pr-benchmarks bot commented Jan 19, 2023

Benchmark started for microbenchmarks on aspnet-perf-win with runtime and arguments --variable filter=*Formatting*. Logs: link

@pr-benchmarks
Copy link

pr-benchmarks bot commented Jan 19, 2023

microbenchmarks - aspnet-perf-win

| benchmark                                                                                        | mean (microbenchmarks.base) | mean (microbenchmarks.pr) | ratio | allocated (microbenchmarks.base) | allocated (microbenchmarks.pr) | ratio |
| ------------------------------------------------------------------------------------------------ | --------------------------- | ------------------------- | ----- | -------------------------------- | ------------------------------ | ----- |
| Microsoft.Extensions.Logging.Formatting.NoArguments                                              |                    21.79 ns |                  21.54 ns |  0.99 |                              0 B |                            0 B |       |
| Microsoft.Extensions.Logging.Formatting.TwoArguments                                             |                   101.75 ns |                  97.92 ns |  0.96 |                             80 B |                           80 B |  1.00 |
| Microsoft.Extensions.Logging.FormattingOverhead.FourArguments_DefineMessage                      |                    161.7 ns |                  161.6 ns |  1.00 |                            176 B |                          176 B |  1.00 |
| Microsoft.Extensions.Logging.FormattingOverhead.FourArguments_EnumerableArgument                 |                    585.0 ns |                  568.1 ns |  0.97 |                            656 B |                          656 B |  1.00 |
| Microsoft.Extensions.Logging.FormattingOverhead.NoArguments                                      |                    19.67 ns |                  19.57 ns |  1.00 |                              0 B |                            0 B |       |
| Microsoft.Extensions.Logging.FormattingOverhead.TwoArguments                                     |                    116.9 ns |                  118.6 ns |  1.01 |                            120 B |                          120 B |  1.00 |
| Microsoft.Extensions.Logging.FormattingOverhead.TwoArguments_DefineMessage                       |                    97.29 ns |                  97.83 ns |  1.01 |                             80 B |                           80 B |  1.00 |
| PerfLabTests.LowLevelPerf.IntegerFormatting                                                      |                    775.6 μs |                  753.1 μs |  0.97 |                          4.58 MB |                        4.58 MB |  1.00 |
| System.Perf_Convert.ToBase64CharArray(binaryDataSize: 1024, formattingOptions: InsertLineBreaks) |                    997.4 ns |                  992.8 ns |  1.00 |                              0 B |                            0 B |       |
| System.Perf_Convert.ToBase64CharArray(binaryDataSize: 1024, formattingOptions: None)             |                    94.64 ns |                  94.82 ns |  1.00 |                              0 B |                            0 B |       |
| System.Perf_Convert.ToBase64String(formattingOptions: InsertLineBreaks)                          |                      1.1 μs |                    1.1 μs |  0.99 |                          2.77 KB |                        2.77 KB |  1.00 |
| System.Perf_Convert.ToBase64String(formattingOptions: None)                                      |                    180.3 ns |                  179.5 ns |  1.00 |                           2.7 KB |                         2.7 KB |  1.00 |

@sebastienros sebastienros mentioned this pull request Jan 19, 2023
@sebastienros
Copy link
Member

/benchmark microbenchmarks aspnet-citrine-win-ampere runtime --variable filter=Formatting

@pr-benchmarks
Copy link

pr-benchmarks bot commented Jan 19, 2023

Benchmark started for microbenchmarks on aspnet-citrine-win-ampere with runtime and arguments --variable filter=*Formatting*. Logs: link

@pr-benchmarks
Copy link

pr-benchmarks bot commented Jan 20, 2023

microbenchmarks - aspnet-citrine-win-ampere

| benchmark                                                                                        | mean (microbenchmarks.base) | mean (microbenchmarks.pr) | ratio | allocated (microbenchmarks.base) | allocated (microbenchmarks.pr) | ratio |
| ------------------------------------------------------------------------------------------------ | --------------------------- | ------------------------- | ----- | -------------------------------- | ------------------------------ | ----- |
| Microsoft.Extensions.Logging.Formatting.NoArguments                                              |                    64.11 ns |                  65.49 ns |  1.02 |                              0 B |                            0 B |       |
| Microsoft.Extensions.Logging.Formatting.TwoArguments                                             |                    260.6 ns |                  283.0 ns |  1.09 |                             80 B |                           80 B |  1.00 |
| Microsoft.Extensions.Logging.FormattingOverhead.FourArguments_DefineMessage                      |                    568.1 ns |                  512.1 ns |  0.90 |                            176 B |                          176 B |  1.00 |
| Microsoft.Extensions.Logging.FormattingOverhead.FourArguments_EnumerableArgument                 |                      1.6 μs |                    1.5 μs |  0.91 |                            656 B |                          656 B |  1.00 |
| Microsoft.Extensions.Logging.FormattingOverhead.NoArguments                                      |                    63.82 ns |                  57.47 ns |  0.90 |                              0 B |                            0 B |       |
| Microsoft.Extensions.Logging.FormattingOverhead.TwoArguments                                     |                    395.3 ns |                  402.2 ns |  1.02 |                            120 B |                          120 B |  1.00 |
| Microsoft.Extensions.Logging.FormattingOverhead.TwoArguments_DefineMessage                       |                    301.2 ns |                  307.0 ns |  1.02 |                             80 B |                           80 B |  1.00 |
| PerfLabTests.LowLevelPerf.IntegerFormatting                                                      |                      2.2 ms |                    2.4 ms |  1.12 |                          4.58 MB |                        4.58 MB |  1.00 |
| System.Perf_Convert.ToBase64CharArray(binaryDataSize: 1024, formattingOptions: InsertLineBreaks) |                      2.0 μs |                    2.0 μs |  1.02 |                              0 B |                            0 B |       |
| System.Perf_Convert.ToBase64CharArray(binaryDataSize: 1024, formattingOptions: None)             |                    965.3 ns |                1,006.8 ns |  1.04 |                              0 B |                            0 B |       |
| System.Perf_Convert.ToBase64String(formattingOptions: InsertLineBreaks)                          |                      2.2 μs |                    2.3 μs |  1.06 |                          2.77 KB |                        2.77 KB |  1.00 |
| System.Perf_Convert.ToBase64String(formattingOptions: None)                                      |                      1.1 μs |                    1.2 μs |  1.03 |                           2.7 KB |                         2.7 KB |  1.00 |

@stephentoub stephentoub merged commit 8798c04 into dotnet:main Jan 20, 2023
@stephentoub stephentoub deleted the compositeformat branch January 20, 2023 10:36
mdh1418 pushed a commit to mdh1418/runtime that referenced this pull request Jan 24, 2023
* Add System.Text.CompositeFormat

* Conditionalize a test and remove extra const

* Address PR feedback
@dotnet dotnet locked as resolved and limited conversation to collaborators Feb 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Boost performance of localized, non-static or incremental string formatting.
6 participants