From 4a5f75f82df141c23304488da84749d07fb070c0 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Sat, 26 Oct 2024 10:52:42 -0700 Subject: [PATCH] Reduce string allocations during SimpleJson.ParseString Noticed this allocation while looking at a profile of solution load in visual studio. These StringBuilder allocations were showing up as 0.5% of total allocations. I took a peek at the code, and reaslized the StringBuilder need not be created unless there is a '\' in the string value being parsed. In that case, just copy already seen characters into a StringBuilder and continue as previously. --- Octokit.Tests/SimpleJsonTests.cs | 36 ++++++++++++++++++++++++++++++++ Octokit/SimpleJson.cs | 25 ++++++++++++++++++---- 2 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 Octokit.Tests/SimpleJsonTests.cs diff --git a/Octokit.Tests/SimpleJsonTests.cs b/Octokit.Tests/SimpleJsonTests.cs new file mode 100644 index 0000000000..ffc211cee4 --- /dev/null +++ b/Octokit.Tests/SimpleJsonTests.cs @@ -0,0 +1,36 @@ +using Octokit; +using System.Threading.Tasks; +using Xunit; + +public class SimpleJsonTests +{ + [Theory] + [InlineData("\"abc\"", "abc")] + [InlineData(" \"abc\" ", "abc")] + [InlineData("\" abc \" ", " abc ")] + [InlineData("\"abc\\\"def\"", "abc\"def")] + [InlineData("\"abc\\r\\ndef\"", "abc\r\ndef")] + public async Task ParseStringSuccess(string input, string expected) + { + int index = 0; + bool success = true; + + string actual = SimpleJson.ParseString(input.ToCharArray(), ref index, ref success); + + Assert.True(success); + Assert.Equal(expected, actual); + } + + [Theory] + [InlineData("\"abc")] + public async Task ParseStringIncomplete(string input) + { + int index = 0; + bool success = true; + + string actual = SimpleJson.ParseString(input.ToCharArray(), ref index, ref success); + + Assert.False(success); + Assert.Null(actual); + } +} diff --git a/Octokit/SimpleJson.cs b/Octokit/SimpleJson.cs index 7b9a855481..bafb26f921 100644 --- a/Octokit/SimpleJson.cs +++ b/Octokit/SimpleJson.cs @@ -792,15 +792,18 @@ static object ParseValue(char[] json, ref int index, ref bool success) return null; } - static string ParseString(char[] json, ref int index, ref bool success) + internal static string ParseString(char[] json, ref int index, ref bool success) { - StringBuilder s = new StringBuilder(BUILDER_CAPACITY); + // Avoid allocating this StringBuilder unless a backslash is encountered in the json + StringBuilder s = null; char c; EatWhitespace(json, ref index); // " c = json[index++]; + + int startIndex = index; bool complete = false; while (!complete) { @@ -815,6 +818,13 @@ static string ParseString(char[] json, ref int index, ref bool success) } else if (c == '\\') { + if (s == null) + { + s = new StringBuilder(BUILDER_CAPACITY); + for (int i = startIndex; i < index - 1; i++) + s.Append(json[i]); + } + if (index == json.Length) break; c = json[index++]; @@ -875,14 +885,21 @@ static string ParseString(char[] json, ref int index, ref bool success) } } else - s.Append(c); + { + if (s != null) + s.Append(c); + } } if (!complete) { success = false; return null; } - return s.ToString(); + + if (s != null) + return s.ToString(); + + return new string(json, startIndex, index - startIndex - 1); } private static string ConvertFromUtf32(int utf32)