Skip to content

Commit

Permalink
fix: Reduce string allocations during SimpleJson.ParseString (#2977)
Browse files Browse the repository at this point in the history
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.

Co-authored-by: Nick Floyd <139819+nickfloyd@users.noreply.github.com>
  • Loading branch information
ToddGrun and nickfloyd authored Jan 8, 2025
1 parent 4f892d6 commit 7c6c08f
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 4 deletions.
36 changes: 36 additions & 0 deletions Octokit.Tests/SimpleJsonTests.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
25 changes: 21 additions & 4 deletions Octokit/SimpleJson.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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++];
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 7c6c08f

Please sign in to comment.