search

bleroy / lunr-core

A port of LUNR.js to .NET Core
MIT License
558 stars 24 forks source link

Introduce ValueStringBuilder to eliminate various allocations #50

Closed iamcarbon closed 1 year ago

iamcarbon commented 2 years ago

This PR introduces and uses the ValueStringBuilder from the dotnet runtime to eliminate a few allocations.

iamcarbon commented 2 years ago

@bleroy Ready for review.

bleroy commented 2 years ago

Thanks for the contribution, this looks interesting, especially if you have perf tests to demonstrate the benefits. A few questions though:

  1. Wouldn't String.Create have a good use case here?
  2. Unclear what happens if you call multiple methods that make a call to Dispose.
  3. Any idea why StringBuilder doesn't have those abilities?

Most of all, however, I think this is way too complex to not come with a test suite and perf benchmarks, I hope that won't be too much trouble adding.