Skip to content

Conversation

@TimothyMakkison
Copy link
Contributor

@TimothyMakkison TimothyMakkison commented Mar 18, 2025

  • Replaces StringBuilder with ValueListBuilder<char>
  • Added some extension methods to make this easier
  • Uses SkipLocalsInit to avoid the cost to zero stackalloc char[]

Benchmarks

I have no idea why, all the new changes I make are slower in the benchmarks, even when they should be a straight upgrade 🤔
I'm running into this issue a lot with #1542

Before

Method Mean Error StdDev Median Gen0 Gen1 Allocated
Default_CodeFormatter_Tests 133.7 ms 2.65 ms 3.54 ms 133.7 ms 2000.0000 1000.0000 31.07 MB
Default_CodeFormatter_Complex 284.9 ms 5.60 ms 13.95 ms 279.1 ms 5000.0000 2000.0000 54.59 MB

After

Method Mean Error StdDev Median Gen0 Gen1 Gen2 Allocated
Default_CodeFormatter_Tests 136.2 ms 2.69 ms 5.74 ms 134.5 ms 2000.0000 1000.0000 - 30.71 MB
Default_CodeFormatter_Complex 285.6 ms 5.71 ms 14.93 ms 278.1 ms 6000.0000 3000.0000 1000.0000 53.45 MB

@belav belav force-pushed the main branch 4 times, most recently from eeaf247 to 9e9ad83 Compare July 10, 2025 15:48
@TimothyMakkison TimothyMakkison force-pushed the VLB_string_builder branch 2 times, most recently from 3ea9bb4 to b7586f4 Compare July 13, 2025 21:26
@TimothyMakkison TimothyMakkison marked this pull request as ready for review July 13, 2025 21:27
Copy link
Owner

@belav belav left a comment

Choose a reason for hiding this comment

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

Did you figure out why your old benchmarks showed this as being slightly slower?

@TimothyMakkison
Copy link
Contributor Author

TimothyMakkison commented Jul 18, 2025

In retrospect I think I was being overly paranoid. 😓 This is a net positive for memory usage with a negligible improvement to speed. The benchmarks naturally vary a little

@TimothyMakkison TimothyMakkison force-pushed the VLB_string_builder branch 2 times, most recently from ade98c7 to c184562 Compare July 23, 2025 17:32
@belav belav merged commit e9d4e75 into belav:main Aug 3, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants