Reduce string allocations during SimpleJson.ParseString #2977
Conversation
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.
|
@kfcampbell -- are you a good person to work with for this? |
|
@kfcampbell -- Is there someone else that is more appropriate to work with on this? |
|
@shiftkey -- can you help me find someone to work with to help this proceed. It's a fairly small allocation improvement that is showing up in VisualStudio profiles. |
|
@shiftkey -- can you provide some guidance for me on how to proceed? |
|
@MatisseHack -- it looks like you recently were able to merge a PR, so it seems like this codebase is still active. Any suggestions on how I can proceed with this? |
…JsonStringParsing
|
@kfcampbell @shiftkey @MatisseHack -@nickfloyd - Can somebody please respond? This is getting frustrating. I've found a performance issue, tracked it down and created a fix, and have been pushing for almost 2 months for someone to respond. Should this codebase not be taken as a dependency? |
|
@JonruAlveus -- Thanks for taking a look. Is there a process to get this merged? |
|
Hey @ToddGrun Apologies for the delay on this. We've unfortunately had limited availability over the past few months to get to our ever growing queue of review items. I'm looking at this now and will review and deploy once I am done looking over this. Thank you for your patience. |
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.
Resolves 2976
Before the change?
After the change?
Pull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!