Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Clean up Uri.UnescapeDataString #42225

Merged
merged 1 commit into from
Oct 31, 2019

Conversation

stephentoub
Copy link
Member

  • Use string.IndexOf rather than an open-coded, unsafe loop.
  • Avoid an unnecessary SequenceEquals at the end: we're only at this point if a % was found highlighting that something escaped was found.
  • Use stack memory for smaller inputs if possible, to avoid unnecessary ArrayPool interaction
  • Remove an unnecessary argument to a helper function.
  • Fix ValueStringBuilder.Grow to only copy the contained data.

This was really meant as a minor cleanup, e.g. to remove unsafe code that needn't be. It does however have a perf benefit for longer inputs when nothing needs to be unescaped (or when the thing that needs to be unescaped is later in the input), as then the use of IndexOf provides vectorization "for free".

Method Toolchain Repeats InputMode Mean Error StdDev Ratio RatioSD Gen 0 Gen 1 Gen 2 Allocated
Unescape \new\corerun.exe 4 ASCII 13.73 ns 0.096 ns 0.085 ns 0.85 0.02 - - - -
Unescape \old\corerun.exe 4 ASCII 16.10 ns 0.265 ns 0.248 ns 1.00 0.00 - - - -
Unescape \new\corerun.exe 4 Emoji 250.76 ns 2.311 ns 1.929 ns 0.92 0.02 0.0367 - - 232 B
Unescape \old\corerun.exe 4 Emoji 273.92 ns 3.279 ns 3.068 ns 1.00 0.00 0.0367 - - 232 B
Unescape \new\corerun.exe 4 Escaped 58.47 ns 0.480 ns 0.425 ns 0.80 0.01 0.0050 - - 32 B
Unescape \old\corerun.exe 4 Escaped 72.96 ns 0.974 ns 0.864 ns 1.00 0.00 0.0050 - - 32 B
Unescape \new\corerun.exe 4 Mixed 84.71 ns 1.392 ns 1.302 ns 0.71 0.02 0.0101 - - 64 B
Unescape \old\corerun.exe 4 Mixed 118.71 ns 1.265 ns 1.183 ns 1.00 0.00 0.0100 - - 64 B
Unescape \new\corerun.exe 40 ASCII 16.75 ns 0.061 ns 0.054 ns 0.19 0.00 - - - -
Unescape \old\corerun.exe 40 ASCII 86.24 ns 0.791 ns 0.740 ns 1.00 0.00 - - - -
Unescape \new\corerun.exe 40 Emoji 1,719.34 ns 4.388 ns 4.105 ns 1.01 0.01 0.2651 - - 1672 B
Unescape \old\corerun.exe 40 Emoji 1,702.36 ns 15.955 ns 14.144 ns 1.00 0.00 0.2651 - - 1672 B
Unescape \new\corerun.exe 40 Escaped 257.49 ns 5.161 ns 5.944 ns 0.97 0.02 0.0162 - - 104 B
Unescape \old\corerun.exe 40 Escaped 265.17 ns 3.782 ns 3.538 ns 1.00 0.00 0.0162 - - 104 B
Unescape \new\corerun.exe 40 Mixed 670.76 ns 6.920 ns 6.473 ns 1.13 0.02 0.0668 - - 424 B
Unescape \old\corerun.exe 40 Mixed 595.95 ns 7.990 ns 7.473 ns 1.00 0.00 0.0668 - - 424 B
Unescape \new\corerun.exe 400 ASCII 63.74 ns 0.579 ns 0.541 ns 0.09 0.00 - - - -
Unescape \old\corerun.exe 400 ASCII 743.76 ns 9.243 ns 8.646 ns 1.00 0.00 - - - -
Unescape \new\corerun.exe 400 Emoji 15,545.16 ns 123.640 ns 115.653 ns 0.96 0.01 2.5330 - - 16072 B
Unescape \old\corerun.exe 400 Emoji 16,178.76 ns 84.439 ns 78.984 ns 1.00 0.00 2.5330 - - 16072 B
Unescape \new\corerun.exe 400 Escaped 2,126.10 ns 40.104 ns 46.184 ns 1.00 0.03 0.1297 - - 824 B
Unescape \old\corerun.exe 400 Escaped 2,123.31 ns 16.610 ns 15.537 ns 1.00 0.00 0.1297 - - 824 B
Unescape \new\corerun.exe 400 Mixed 5,248.00 ns 43.814 ns 38.840 ns 0.97 0.01 0.6409 0.0076 - 4024 B
Unescape \old\corerun.exe 400 Mixed 5,391.88 ns 62.970 ns 58.902 ns 1.00 0.00 0.6409 0.0076 - 4024 B
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Diagnosers;
using BenchmarkDotNet.Running;
using System;
using System.Linq;

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

    [Params(4, 40, 400)] public int Repeats { get; set; }

    [Params("ASCII", "Mixed", "Escaped", "Emoji")] public string InputMode { get; set; }

    private string _value;

    [GlobalSetup]
    public void Setup() =>
        _value = InputMode switch
        {
            "ASCII" => string.Concat(Enumerable.Repeat("abcd", Repeats)),
            "Mixed" => string.Concat(Enumerable.Repeat("abcd%2B", Repeats)),
            "Escaped" => string.Concat(Enumerable.Repeat("%2B", Repeats)),
            "Emoji" => string.Concat(Enumerable.Repeat("%F0%9F%98%80", Repeats)),
            _ => throw new Exception()
        };

    [Benchmark]
    public string Unescape() => Uri.UnescapeDataString(_value);
}

- Use string.IndexOf rather than an open-coded, unsafe loop.
- Avoid an unnecessary SequenceEquals at the end: we're only at this point if a `%` was found highlighting that something escaped was found.
- Use stack memory for smaller inputs if possible, to avoid unnecessary ArrayPool interaction
- Remove an unnecessary argument to a helper function.
- Fix ValueStringBuilder.Grow to only copy the contained data.
Copy link
Member

@benaadams benaadams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much cleaner

@davidsh davidsh added this to the 5.0 milestone Oct 30, 2019
@stephentoub stephentoub merged commit 04f79d9 into dotnet:master Oct 31, 2019
@stephentoub stephentoub deleted the cleanupunescapedata branch October 31, 2019 00:00
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/coreclr that referenced this pull request Oct 31, 2019
- Use string.IndexOf rather than an open-coded, unsafe loop.
- Avoid an unnecessary SequenceEquals at the end: we're only at this point if a `%` was found highlighting that something escaped was found.
- Use stack memory for smaller inputs if possible, to avoid unnecessary ArrayPool interaction
- Remove an unnecessary argument to a helper function.
- Fix ValueStringBuilder.Grow to only copy the contained data.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corert that referenced this pull request Oct 31, 2019
- Use string.IndexOf rather than an open-coded, unsafe loop.
- Avoid an unnecessary SequenceEquals at the end: we're only at this point if a `%` was found highlighting that something escaped was found.
- Use stack memory for smaller inputs if possible, to avoid unnecessary ArrayPool interaction
- Remove an unnecessary argument to a helper function.
- Fix ValueStringBuilder.Grow to only copy the contained data.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/mono that referenced this pull request Oct 31, 2019
- Use string.IndexOf rather than an open-coded, unsafe loop.
- Avoid an unnecessary SequenceEquals at the end: we're only at this point if a `%` was found highlighting that something escaped was found.
- Use stack memory for smaller inputs if possible, to avoid unnecessary ArrayPool interaction
- Remove an unnecessary argument to a helper function.
- Fix ValueStringBuilder.Grow to only copy the contained data.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
akoeplinger pushed a commit to mono/mono that referenced this pull request Oct 31, 2019
- Use string.IndexOf rather than an open-coded, unsafe loop.
- Avoid an unnecessary SequenceEquals at the end: we're only at this point if a `%` was found highlighting that something escaped was found.
- Use stack memory for smaller inputs if possible, to avoid unnecessary ArrayPool interaction
- Remove an unnecessary argument to a helper function.
- Fix ValueStringBuilder.Grow to only copy the contained data.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas pushed a commit to dotnet/coreclr that referenced this pull request Oct 31, 2019
- Use string.IndexOf rather than an open-coded, unsafe loop.
- Avoid an unnecessary SequenceEquals at the end: we're only at this point if a `%` was found highlighting that something escaped was found.
- Use stack memory for smaller inputs if possible, to avoid unnecessary ArrayPool interaction
- Remove an unnecessary argument to a helper function.
- Fix ValueStringBuilder.Grow to only copy the contained data.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas pushed a commit to dotnet/corert that referenced this pull request Oct 31, 2019
- Use string.IndexOf rather than an open-coded, unsafe loop.
- Avoid an unnecessary SequenceEquals at the end: we're only at this point if a `%` was found highlighting that something escaped was found.
- Use stack memory for smaller inputs if possible, to avoid unnecessary ArrayPool interaction
- Remove an unnecessary argument to a helper function.
- Fix ValueStringBuilder.Grow to only copy the contained data.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
- Use string.IndexOf rather than an open-coded, unsafe loop.
- Avoid an unnecessary SequenceEquals at the end: we're only at this point if a `%` was found highlighting that something escaped was found.
- Use stack memory for smaller inputs if possible, to avoid unnecessary ArrayPool interaction
- Remove an unnecessary argument to a helper function.
- Fix ValueStringBuilder.Grow to only copy the contained data.

Commit migrated from dotnet/corefx@04f79d9
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.

4 participants