crozone / FormatWith

String extensions for named parameterized string formatting.
MIT License
73 stars 13 forks source link

Add Span-based implementation and netstandard2.1 target framework #20

Closed JeremySkinner closed 3 years ago

JeremySkinner commented 4 years ago

Hi @crozone

This is an initial Span-based implementation of FormatWith (#8). Please feel free to use as much/little of it as you like (or discard completely if it doesn't fit with the way you want to do this!)

Here's a summary of what I've done:

FormatToken struct

Tokenizer updates

The method signature is now:

public static void Tokenize(ReadOnlySpan<char> formatString, SpanAction action, char openBraceChar = '{', char closeBraceChar = '}')

...where SpanAction is defined as internal delegate void SpanAction(FormatToken token) (can't use Action<FormatToken> as ref structs can't be used as type parameters).

ProcessTokens / ProcessTokensIntoFormattableString

Backwards compatibility with netstandard2

For netstandard2.1, Span is native, as are methods such as StringBuilder.Append(Span<char>).

For netstandard2, the System.Memory package can be used to add Span support to older frameworks. This has the caveat that StringBuilder doesn't have an overload that accepts Span, but I hacked this in with an extension method with #ifdefs.

The advantage of this approach is very little conditional compilation is necessary, as the API is globally working with spans. The downside is that using Span on older frameworks is probably actually going to be slower than your original implementation. This may be an acceptable tradeoff, or you might feel it's better to to stick with the old approach when running under netstandard2.0, and have more conditional compilation instead. Let me know what you think.

Further work

There are a couple of areas that could be optimised further, but wanted to let you have a look before I do any more on this.

Edit: I also notice you don't have continuous integration set up on here. I'd be happy to do the work to add that in for you with github actions, if you'd like it.

JeremySkinner commented 4 years ago

A very quick informal benchmark for comparison. Full test run on my laptop, compiled in release mode (not using any proper benchmarking tools)

Existing netstandard2.0 implementation: 7s Span-based netstandard2.0 implementation: 9s Span-based netstandard2.1 implementation: 6s

I'll do some proper benchmarks at some point.

crozone commented 4 years ago

Hey @JeremySkinner, this is awesome!

I am probably going to use most of this. I really like the way the tokenizer works, I was working on a slightly different approach to eliminate the IEnumerable but I think I'll use yours instead because the dependency inversion with the SpanAction action is quite elegant.

I'm still working on the public API, I'm thinking of making the "Format Handler" action itself take an action which you can pass a span into, which is then written into the string builder (similar to how the tokenizer works but another level up). FormatToken is probably also going to need a little work.

I think we can also get away with making the StringBuilder a [ThreadStatic] which might make the entire thing almost zero allocating, which is quite exciting.

As for CI, I've never worked with github actions before - if you would be willing to set that up that would be fantastic.

Cheers!

JeremySkinner commented 4 years ago

Great! Give me a shout if there's anything you want me to change in this PR, and I'll send you a separate one to enable CI.