elps icon indicating copy to clipboard operation
elps copied to clipboard

RFC - `format` implementation

Open reubent opened this issue 4 years ago • 0 comments

The implementation of format is cool but probably overcomplex for what it actually does. There's also a TODO to add positional parameters.

I've written a (much) simpler implementation that supports positional parameters. You'll find it in format_test.go as quickformatBuiltin at the moment. I've also written a series of tests and benchmarks for both implementations.

As it stands, there is code there to support unmatched brackets, but the more I think about it, the less desirable I think that actually is. Mistakes are obvious because the number of brackets won't match the number of parameters and this gives a way to use a literal curly bracket. Considering the performance impact of it as well, this seems reasonable.

With unmatched checking turned off, performance for this version is around 20% better for simple strings:

BenchmarkQuickFormatString
BenchmarkQuickFormatString-8                      	  727240	      1533 ns/op

versus

BenchmarkFormatString
BenchmarkFormatString-8                           	  631561	      1887 ns/op

With a lot of tokens, the gap is a lot wider:

BenchmarkQuickFormatStringWithATonOfTokens
BenchmarkQuickFormatStringWithATonOfTokens-8      	  116503	     10249 ns/op

versus

BenchmarkFormatStringWithATonOfTokens
BenchmarkFormatStringWithATonOfTokens-8           	   69565	     16725 ns/op

Performance is marginally slower with positional tokens, but not a lot:

BenchmarkQuickFormatStringWithPositionalToken
BenchmarkQuickFormatStringWithPositionalToken-8   	  727245	      1666 ns/op

Turning on the simple bad token check, performance drops to:

BenchmarkQuickFormatString
BenchmarkQuickFormatString-8                      	  599984	      1723 ns/op
BenchmarkQuickFormatStringWithATonOfTokens
BenchmarkQuickFormatStringWithATonOfTokens-8      	   88561	     13319 ns/op

and if I use the "comprehensive" check:

BenchmarkQuickFormatString
BenchmarkQuickFormatString-8                      	  380971	      2916 ns/op
BenchmarkQuickFormatStringWithATonOfTokens
BenchmarkQuickFormatStringWithATonOfTokens-8      	   18986	     63204 ns/op

reubent avatar Jan 30 '21 10:01 reubent