Optimise HtmlEncode and HtmlDecode to produce less garbage by benjamin-hodgson · Pull Request #27250 · dotnet/corefx

benjamin-hodgson · 2018-02-19T14:27:36Z

This fixes https://github.com/dotnet/corefx/issues/13893. I'd noticed in a project of mine that the TextWriter overload of WebUtility.HtmlEncode was unexpectedly generating garbage.

I added a System.Buffers project reference to the System.Runtime.Extensions csproj and wrote a simple implementation of StringBuilder based on ArrayPool, as suggested by @jkotas in #13875.

I benchmarked this implementation and while it appears to produce no garbage (except for the string that it returns) it does seem to run around 10-20% slower. I'm keen to get some input from you guys on what might be going on here.

Benchmark code (using BenchmarkDotNet):

[MemoryDiagnoser]
public class Bench
{
    public static readonly string _toEncode = string.Concat(Enumerable.Repeat("<>\"'", 100));

    [Benchmark(Baseline = true)]
    public void String_Old()
    {
        WebUtility.HtmlEncode(_toEncode);
    }

    [Benchmark]
    public void String_New()
    {
        WebUtility.HtmlEncodeNew(_toEncode);
    }

    [Benchmark]
    public void TextWriter_Old()
    {
        using (var writer = TextWriter.Null)
        {
            WebUtility.HtmlEncode(_toEncode, writer);
        }
    }

    [Benchmark]
    public void TextWriter_New()
    {
        using (var writer = TextWriter.Null)
        {
            WebUtility.HtmlEncodeNew(_toEncode, writer);
        }
    }
}

And the results:

         Method |     Mean |     Error |    StdDev | Scaled | ScaledSD |  Gen 0 |  Gen 1 | Allocated |
--------------- |---------:|----------:|----------:|-------:|---------:|-------:|-------:|----------:|
     String_Old | 5.394 us | 0.0987 us | 0.0924 us |   1.00 |     0.00 | 2.0828 | 0.0229 |   13120 B |
     String_New | 6.571 us | 0.1101 us | 0.1030 us |   1.22 |     0.03 | 0.7629 |      - |    4832 B |
 TextWriter_Old | 5.431 us | 0.1078 us | 0.1153 us |   1.01 |     0.03 | 2.0828 | 0.0229 |   13120 B |
 TextWriter_New | 6.080 us | 0.1041 us | 0.0974 us |   1.13 |     0.03 |      - |      - |       0 B |

jkotas · 2018-02-19T14:50:37Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

-            HtmlEncode(value, index, sb);
-            return StringBuilderCache.GetStringAndRelease(sb);
+            var sb = new PooledStringBuilder(value.Length);
+            try


You do not need try/catch. It is ok to turn the pooled into garbage on exceptions.

jkotas · 2018-02-19T14:53:05Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs


 #endregion

+        private struct PooledStringBuilder


We have a helper class like this here: https://github.com/dotnet/corefx/blob/master/src/Common/src/CoreLib/System/Text/ValueStringBuilder.cs that has a few more optimizations. Try using that instead.

It looks like ValueStringBuilder is backed by a Span - what's the best way of writing a Span<char> to a TextWriter? I don't want to just loop over the span and call Write because that'll be slow, and I don't want to copy the span into an array because that'll generate garbage.

void Write(System.ReadOnlySpan<char> buffer) method.

stephentoub · 2018-02-19T16:08:08Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

                return value;
            }

-            StringBuilder sb = StringBuilderCache.Acquire(value.Length);


Is the project still using StringBuilderCache after this change? If not, it can be removed from the .csproj. If yes, should those places be changed to ValueStringBuilder as well?

It's used elsewhere (eg). I'd be happy to fix those other usages too, in a separate PR, if you like.

Hi! Other usages have been removed in #30072

stephentoub · 2018-02-19T16:09:19Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

+            {
+                output.Write(value);
+                return;
+            }


How valuable is this check? In cases where we do have something to encode, we end up paying for the extra checking.

I just copied this from the existing string overload of HtmlEncode above. My gut tells me that the case when value doesn't have any encoding characters is quite common, probably more common than when it does. Plus index is used to copy the prefix of the string quickly, see line 106

davidfowl · 2018-02-19T16:18:06Z

FYI there's a much more efficient HtmlEncoder implementation here https://github.com/dotnet/corefx/blob/01fa16ffd618846a913cad719269c8bb441ceb28/src/System.Text.Encodings.Web/src/System/Text/Encodings/Web/HtmlEncoder.cs.

jkotas · 2018-02-19T16:33:20Z

src/Common/src/CoreLib/System/Text/ValueStringBuilder.cs

            }
        }

+        public void WriteTo(TextWriter writer)


This method is very specialized. It should rather be .AsReadOnlySpan() property. It will handle all future uses that need to get the string as Span.

Are you sure? That sounds like it could be a bit risky: when the underlying array gets resized the old one gets returned to the pool but the ReadOnlySpan will still have a reference to it.

This is internal type, not a public API. There are many ways you can use it in a wrong way. For example, you can accidentally pass it by value and create a second copy of it to introduce the same problem as if you use AsReadOnlySpan incorrectly by accident.

benjamin-hodgson · 2018-02-19T16:43:36Z

@davidfowl Thanks, I didn't know about HtmlEncoder.

jkotas · 2018-02-19T17:01:26Z

src/Common/src/CoreLib/System/Text/ValueStringBuilder.cs


        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private void Clear()
+        public void Clear()


Nit: It may be better to call it Dispose now that it is public.

I assume going further and adding IDisposable would be too heavyweight for this -- especially since "leaking" is not a big deal.

Right. Also, interfaces do not really make sense for ref types - you cannot cast the ref type to the interface.

A tangent: I haven't tried this, but I imagine a ref struct with an interface might have some limited use in the context of constrained generics. That is, if you had

ref struct Foo : IBar {} void Baz<T>(T baz) where T : IBar

you should be able to write

Baz(new Foo());

Although: I guess you're probably not allowed to use a ref struct as a type argument, because you don't know whether the method will use the T in a way that you can't use a ref struct (eg casting it to object).

jkotas · 2018-02-19T17:03:43Z

src/System.Runtime.Extensions/src/System.Runtime.Extensions.csproj

    <ProjectReference Include="..\..\System.Diagnostics.Debug\src\System.Diagnostics.Debug.csproj" />
    <ProjectReference Include="..\..\System.Security.Principal\src\System.Security.Principal.csproj" />
    <ProjectReference Include="..\..\System.Runtime\src\System.Runtime.csproj" />
+    <ProjectReference Include="..\..\System.Buffers\src\System.Buffers.csproj" />


Do you actually need to add this reference? Span is in System.Runtime for netcoreapp - you do not need to reference System.Buffers to get it.

Ah, good catch, thanks! I needed in an earlier commit, because I was using ArrayPool directly, but that's been removed now.

danmoseley · 2018-02-21T01:51:08Z

src/Common/src/CoreLib/System/Text/ValueStringBuilder.cs

            _chars = initialBuffer;
            _pos = 0;
        }
+        public ValueStringBuilder(int initialCapacity)


Can you please add some test(s) to ValueStringBuilderTests for your new functionality?

danmoseley · 2018-02-21T18:37:24Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

+
+            if (string.IsNullOrEmpty(value))
+            {
+                output.Write(value);


This will presumably throw if value is null. But, I see previously it would have also thrown doing value.Length.

Seems we should probably throw ANE if value is null.

Good spot, will do.

I'm now throwing an ArgumentNullException, but I'm concerned that this is technically a breaking change (see my comment). What do you think?

We are generally OK with changing NRE->ANE.

Oh I see you changed back. That's fine too.

danmoseley · 2018-02-21T18:40:44Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

+                return;
+            }
+
+            if (!StringRequiresHtmlDecoding(value))


The code was not previously doing this check. If we have to do it, it might be nice for StringRequiresHtmlDecoding to return the index of the first character to decode, so HtmlDecode can copy up to that point verbatim, the same optimization happening with IndexOfHtmlEncodingChars.
Without that, I am not sure whether this will make it slower or not.

It was doing this check before, in the HtmlDecode(string value) method (which was being called by this one).

Ah right. Nevertheless it seems that HtmlDecode could have the same ability to start at an index returned from StringRequiresHtmlDecoding. Future change maybe.

danmoseley

As noted. Thanks!

ahsonkhan · 2018-02-24T21:12:27Z

src/Common/tests/Tests/System/Text/ValueStringBuilderTests.cs

+                vsb.Append(s);
+            }
+
+            var resultSpan = vsb.AsReadOnlySpan();


We renamed AsReadOnlySpan to AsSpan.

Also, nit: we don't use resultSpan anywhere. Change to:
var resultString = new string(vsb.AsSpan());

Done, thanks!

ahsonkhan · 2018-02-24T21:14:46Z

src/Common/src/CoreLib/System/Text/ValueStringBuilder.cs

            _chars = initialBuffer;
            _pos = 0;
        }
+        public ValueStringBuilder(int initialCapacity)


nit: add new line between the ctors

Done, thanks!

ahsonkhan

Otherwise, lgtm

jkotas · 2018-02-24T21:23:11Z

src/Common/src/CoreLib/System/Text/ValueStringBuilder.cs


 using System.Buffers;
 using System.Diagnostics;
+using System.IO;


This using should not be needed anymore.

Done, thanks

jkotas · 2018-02-24T21:28:19Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

+            sb.Dispose();
        }

        [SuppressMessage("Microsoft.Usage", "CA1806:DoNotIgnoreMethodResults", MessageId = "System.UInt16.TryParse(System.String,System.Globalization.NumberStyles,System.IFormatProvider,System.UInt16@)", Justification = "UInt16.TryParse guarantees that result is zero if the parse fails.")]


Since you are optimizing allocations, you can also replace uint.TryParse(value.Substring below with uint.TryParse(value.AsSpan below; and delete this SuppressMessage - it does not seem to be relevant anymore.

Done, good spot, thanks!

benjamin-hodgson · 2018-03-07T17:51:03Z

Rebased. Apologies for the delay in getting this back up to speed.

The tests were failing because I changed the code to throw ArgumentNullException when the input string was null, per @danmosemsft's suggestion. Previously HtmlEncode/HtmlDecode would return null (or write nothing to the TextWriter, depending on which overload you called) if the input was null. For now I've updated the tests to match the new behaviour, but this is technically a breaking change; it's likely that client code is relying on being able to pass null to that method. Do you think I should keep the new behaviour or put it back to how it was?

ahsonkhan · 2018-03-08T19:20:23Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

 using System.IO;
 using System.Runtime.CompilerServices;
 using System.Text;
+using System.Buffers;


nit: sort in alphabetical order

I think this import isn't used any more actually. Good catch, thanks!

ahsonkhan · 2018-03-09T21:32:40Z

@benjamin-hodgson, the System.Web.Tests.HttpUtilityTest/HtmlEncode_TextWriter(decoded: null, encoded: "") test is failing in CI

https://mc.dot.net/#/user/benjamin-hodgson/pr~2Fjenkins~2Fdotnet~2Fcorefx~2Fmaster~2F/test~2Ffunctional~2Fcli~2F/127d2ab60cced49647237cd9c6774f5102fb1095/workItem/System.Web.HttpUtility.Tests/analysis/xunit/System.Web.Tests.HttpUtilityTest~2FHtmlEncode_TextWriter(decoded:%20null,%20encoded:%20%5C%22%5C%22)

Message :
System.ArgumentNullException : Value cannot be null.
Parameter name: value
Stack Trace :
   at System.Net.WebUtility.HtmlEncode(String value) in D:\j\workspace\windows-TGrou---74aa877a\src\System.Runtime.Extensions\src\System\Net\WebUtility.cs:line 42
   at System.Web.Util.HttpEncoder.HtmlEncode(String value, TextWriter output) in D:\j\workspace\windows-TGrou---74aa877a\src\System.Web.HttpUtility\src\System\Web\Util\HttpEncoder.cs:line 139
   at System.Web.HttpUtility.HtmlEncode(String s, TextWriter output) in D:\j\workspace\windows-TGrou---74aa877a\src\System.Web.HttpUtility\src\System\Web\HttpUtility.cs:line 140
   at System.Web.Tests.HttpUtilityTest.HtmlEncode_TextWriter(String decoded, String encoded) in D:\j\workspace\windows-TGrou---74aa877a\src\System.Web.HttpUtility\tests\HttpUtility\HttpUtilityTest.cs:line 273

benjamin-hodgson · 2018-03-09T21:37:38Z

@ahsonkhan Yep I know. I think I'm going to undo the ArgumentNullException change because it's a breaking change (see my earlier comment). I haven't had the time yet though, will try and get to it over the weekend.

ViktorHofer · 2018-06-03T14:07:57Z

I'm looking into further optimizations in the WebUtility class. Please stand by.

benjamin-hodgson · 2018-06-03T14:55:46Z

@ViktorHofer Ah thanks very much for looking into that. Those results look really good - is the improvement due to https://github.com/dotnet/coreclr/issues/18005 ? If so should #29921 be closed now?

danmoseley

Maybe open an issue for htmldecode to be passed the index found by StringRequiresHtmlDecodin, as an optimization

ViktorHofer · 2018-06-03T22:13:18Z

The latest commits remove another string allocation and also unnecessary unsafe code where boundary checks are eliminated by the CLR. I benchmarked with my commits and without them and they improve throughput by about 10%.

I noticed that the test string wasn't really very expressive and changed it to something which hits all the different paths in the implementation: Hello! '\"<&>\u2665\u00E7 World \u0021\u0023\u003D\u003F <>\"\\&. Unfortunately with this test string I see the big regression @benjamin-hodgson talked about. See below...

Before

Method	Mean	Error	StdDev	Gen 0	Allocated
EncodeString	49.16 ms	0.9494 ms	1.1659 ms	19062.5000	38.15 MB
EncodeStringAndTextWriter	53.59 ms	0.3883 ms	0.3442 ms	19062.5000	38.15 MB

After

Method	Mean	Error	StdDev	Gen 0	Allocated
EncodeString	75.99 ms	0.3655 ms	0.3052 ms	16000.0000	33600000 B
EncodeStringAndTextWriter	97.22 ms	1.0154 ms	0.8479 ms	-	0 B

ViktorHofer

Big regression (see results)

ahsonkhan · 2018-06-03T22:15:25Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

+                        output.Append("&#");
+
+                        // Use the buffer directly and reserve a conservative estimate of 10 chars.
+                        Span<char> s = output.AppendSpan(10);


nit: avoid single character variable names.

ahsonkhan · 2018-06-03T22:19:08Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

-                            {
-                                parsedSuccessfully = uint.TryParse(value.AsSpan(entityOffset + 1, entityLength - 1), NumberStyles.Integer, CultureInfo.InvariantCulture, out parsedValue);
-                            }
+                            bool parsedSuccessfully = value[entityOffset + 1] == 'x' || value[entityOffset + 1] == 'X'


This is difficult to parse and read. Can we cache some of these in a local? Like value[entityOffset + 1]?

jkotas · 2018-06-03T22:19:16Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

+            var sb = new ValueStringBuilder(value.Length);
+
+            // Prepend non encoding part
+            ReadOnlySpan<char> prefixSpan = value.AsSpan(0, index);


Nit: I think it would look better without an extra local:

// Prepend non encoding part sb.Append(value.AsSpan(0, index));

ahsonkhan · 2018-06-03T22:23:06Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

        }

-        private static unsafe int IndexOfHtmlEncodingChars(string s, int startPos)
+        private static int IndexOfHtmlEncodingChars(string input)


Can we use ReadOnlySpan<char> here instead of string? Especially since we already call value.AsSpan multiple times at the call site.

jkotas · 2018-06-03T22:27:54Z

big regression

What do the numbers look like for Decoding?

ValueStringBuilder that is created for encoding is guaranteed to have insufficient size (modulo bucketing done by ArrayPool) because of encoding always grows. Try changing the line that creates the ValueStringBuilder to var sb = new ValueStringBuilder(value.Length + 200) and see whether it makes any difference.

ViktorHofer · 2018-06-03T22:35:47Z

ValueStringBuilder that is created for encoding is guaranteed to have insufficient size (modulo bucketing done by ArrayPool) because of encoding always grows. Try changing the line that creates the ValueStringBuilder to var sb = new ValueStringBuilder(value.Length + 200) and see whether it makes any difference.

15ms average improvement! Nice!

ViktorHofer · 2018-06-03T22:50:20Z

With @jkotas suggestion of increasing the buffer size (15ms) and switching to stackalloc for small inputs (10ms) I was able to make it faster than it currently is in master.

Before (master)

Method	Mean	Error	StdDev	Gen 0	Allocated
EncodeString	48.99 ms	0.9706 ms	1.1178 ms	19062.5000	38.15 MB
EncodeStringAndTextWriter	50.45 ms	0.9785 ms	1.0469 ms	19062.5000	38.15 MB
DecodeString	81.27 ms	0.4768 ms	0.4227 ms	5812.5000	11.75 MB
DecodeStringAndTextWriter	83.12 ms	1.1897 ms	1.1129 ms	5812.5000	11.75 MB

After (update: latest commit now)

Method	Mean	Error	StdDev	Gen 0	Allocated
EncodeString	45.90 ms	0.1646 ms	0.1374 ms	16000.0000	33600000 B
EncodeStringAndTextWriter	51.24 ms	0.6761 ms	0.6324 ms	-	0 B
DecodeString	74.11 ms	0.4565 ms	0.4270 ms	5812.5000	12320000 B
DecodeStringAndTextWriter	81.72 ms	1.0488 ms	0.9811 ms	-	0 B

jkotas · 2018-06-03T23:07:12Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

-            HtmlDecode(value, sb);
-            return StringBuilderCache.GetStringAndRelease(sb);
+            char[] rawBuffer = null;
+            // We expect the encoded pattern to be at least twice as large as the decoded one.


I think that this is wrong assumption. I think that there are going to be just a very few escaped characters in the typical case.

okay, I will trim the check down to 256. do you think the 80 for the encode check is conservative enough or should we maybe even increase it to 100 / 128?

jkotas · 2018-06-03T23:15:49Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

-            HtmlDecode(value, sb);
-            return StringBuilderCache.GetStringAndRelease(sb);
+            char[] rawBuffer = null;
+            Span<char> buffer = value.Length < 256 ?


This can be value.Length <= 256

ViktorHofer

There's not much point in approving my own changes but here I go

ViktorHofer · 2018-06-03T23:18:27Z

Should we remove the ValueStringBuilder ctor overload with its tests now that we are not using it anymore or keep it as it might bring value for future scenarios?

jkotas · 2018-06-04T00:00:28Z

Should we remove the ValueStringBuilder ctor overload with its tests now that we are not using it anymore

I think so.

ViktorHofer · 2018-06-04T01:13:28Z

Thanks @benjamin-hodgson for your work, I'm done my changes here now. Both throughput and allocation figures are now better than on master therefore I believe this PR is now in a mergeable state. Thanks @jkotas and others for hints.

@stephentoub it would be great if you could glance over the changes before we merge this. thanks!

stephentoub · 2018-06-04T01:15:35Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

+            char[] rawBuffer = null;
+            Span<char> buffer = value.Length < 80 ?
+                stackalloc char[256] :
+                (rawBuffer = ArrayPool<char>.Shared.Rent(value.Length + 200));


A comment on how these constants were chosen would be helpful. In particular, I don't understand the relationship between 80 <-> 256 and value.Length <-> value.Length + 200. Do we have any corpus of inputs we can use to determine good estimates here, e.g. that on average 1% of characters need to be encoded, or something similar, and then do the math on value.Length?

I added comments to explain why we chose these values. The fixed 200 additional characters could/should be transformed into a relative value but for that we first need more information on the distribution of input strings for HtmlEncode & HtmlDecode. See offline thread with Lee.

I want to merge this in its current state as getting data might take some time. We can refine the buffer sizes afterwards.

stephentoub · 2018-06-04T01:28:30Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

+            if (rawBuffer != null)
+            {
+                ArrayPool<char>.Shared.Return(rawBuffer);
+            }


Why are we doing it this way rather than the previous approach of passing the initial capacity into VSB and having it do the ArrayPool access? With this approach, we're going to hold onto this ArrayPool buffer no matter how many times the VSB grows, whereas if the VSB did it, it would return the original buffer to the pool when it first grew out of it.

stephentoub · 2018-06-04T01:29:08Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

+            Span<char> buffer = value.Length < 80 ?
+                stackalloc char[256] :
+                (rawBuffer = ArrayPool<char>.Shared.Rent(value.Length + 200));
+            var sb = new ValueStringBuilder(buffer);


Same comments/questions as above.

stephentoub · 2018-06-04T01:34:38Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

+                        // Use the buffer directly and reserve a conservative estimate of 10 chars.
+                        Span<char> encodingBuffer = output.AppendSpan(10);
+                        valueToEncode.TryFormat(encodingBuffer, out int charsWritten, provider: CultureInfo.InvariantCulture);
+                        output.Length -= (10 - charsWritten);


I think it'd be helpful to put the 10 that's used multiple times into a const that describes how it was picked, e.g. const int MaxInt32Digits = 10 or something like that. I assume valueToEncode can't be negative at this point? If it could, max digits would need to be 11.

stephentoub · 2018-06-04T01:35:18Z

src/System.Runtime.Extensions/src/System/Net/WebUtility.cs

+
+                        // Use the buffer directly and reserve a conservative estimate of 10 chars.
+                        Span<char> encodingBuffer = output.AppendSpan(10);
+                        valueToEncode.TryFormat(encodingBuffer, out int charsWritten, provider: CultureInfo.InvariantCulture);


It shouldn't be necessary to specify the culture here, and doing so will make it slower.

do the same optimisation for HtmlDecode remove now-unneeded reference Add tests for new ValueStringBuilder methods

Reduce allocation and remove unsafe code Use bigger buffer size and use stackallocation for small inputs Optimize decode code path and output writing

ViktorHofer · 2018-06-04T13:00:45Z

@benjamin-hodgson I squashed yours commits together so that the PR is ready to be merged, not squashed.

benjamin-hodgson · 2018-06-04T13:50:24Z

@ViktorHofer OK, no problem. Thanks!

jkotas reviewed Feb 19, 2018

View reviewed changes

stephentoub reviewed Feb 19, 2018

View reviewed changes

jkotas reviewed Feb 19, 2018

View reviewed changes

benjamin-hodgson changed the title ~~Optimise HtmlEncode to produce less garbage~~ Optimise HtmlEncode and HtmlDecode to produce less garbage Feb 19, 2018

jkotas reviewed Feb 19, 2018

View reviewed changes

danmoseley reviewed Feb 21, 2018

View reviewed changes

danmoseley suggested changes Feb 21, 2018

View reviewed changes

danmoseley mentioned this pull request Feb 21, 2018

RegexPrefix struct, RegexFCD buffer to ValueListBuilder & ArrayPool #27314

Merged

ahsonkhan reviewed Feb 24, 2018

View reviewed changes

ahsonkhan approved these changes Feb 24, 2018

View reviewed changes

jkotas reviewed Feb 24, 2018

View reviewed changes

ahsonkhan assigned benjamin-hodgson Feb 24, 2018

ahsonkhan added the area-System.Net label Feb 24, 2018

karelz assigned rmkerr, davidsh, caesar-chen and wfurt Mar 7, 2018

ahsonkhan reviewed Mar 8, 2018

View reviewed changes

danmoseley approved these changes Jun 3, 2018

View reviewed changes

ViktorHofer suggested changes Jun 3, 2018

View reviewed changes

ahsonkhan reviewed Jun 3, 2018

View reviewed changes

jkotas reviewed Jun 3, 2018

View reviewed changes

ahsonkhan reviewed Jun 3, 2018

View reviewed changes

jkotas reviewed Jun 3, 2018

View reviewed changes

ViktorHofer approved these changes Jun 3, 2018

View reviewed changes

stephentoub reviewed Jun 4, 2018

View reviewed changes

Benjamin Hodgson and others added 2 commits June 4, 2018 14:57

Optimise HtmlEncode to produce less garbage

2c19d96

do the same optimisation for HtmlDecode remove now-unneeded reference Add tests for new ValueStringBuilder methods

Remove HtmlCoder allocations & StringBuilderCache

eded8f8

Reduce allocation and remove unsafe code Use bigger buffer size and use stackallocation for small inputs Optimize decode code path and output writing

ViktorHofer added the * NO SQUASH * The PR should not be squashed label Jun 4, 2018

ViktorHofer merged commit 2b8d68b into dotnet:master Jun 4, 2018

benjamin-hodgson mentioned this pull request Jan 31, 2020

ValueStringBuilder is slower at appending short strings than StringBuilder dotnet/runtime#26284

Closed

Conversation

benjamin-hodgson commented Feb 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benjamin-hodgson Feb 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidfowl commented Feb 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benjamin-hodgson Feb 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benjamin-hodgson commented Feb 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danmoseley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

benjamin-hodgson commented Feb 19, 2018 •

edited

Loading

benjamin-hodgson Feb 19, 2018 •

edited

Loading

benjamin-hodgson Feb 19, 2018 •

edited

Loading

benjamin-hodgson commented Mar 7, 2018 •

edited

Loading