stringer command and generated String() function

[Vincent Blanchon]

Hello,

I was wondering why the stringer command has been implemented that way:

const _Pill_name = "PlaceboAspirinIbuprofen"

var _Pill_index = [...]uint8{0, 7, 14, 23}

func (i Pill) String() string {
   if i < 0 || i >= Pill(len(_Pill_index)-1) {
      return "Pill(" + strconv.FormatInt(int64(i), 10) + ")"
   }
   return _Pill_name[_Pill_index[i]:_Pill_index[i+1]]
}

When it could be simpler and faster with a simple enum:

func (i Pill) String() string {
   switch i {
   case 0: return "Placebo"
   case 1: return "Aspirin"
   case 2: return "Ibuprofen"
   case 3: return "Paracetamol"
   default: return "Pill(" + strconv.FormatInt(int64(i), 10) + ")"
   }
}

After running a benchmark with a higher number of values (20), the enum is always faster. I also was surprised to see the binary is not bigger (maybe it will be with more values).

Does someone know what advantages the current design brings?

[Brian Candler]

Could you provide a link to where you found this code?  I imagine it was done that way just for demonstrating some features of go, such as slicing and array lookups.

A simpler and faster way again would be to use a map.

https://play.golang.org/p/ntjhesMsSA9

[Brian Candler]

Or, indeed, an array of strings.

https://play.golang.org/p/3Eg9va4Krqo

[Jan Mercl]

I don't see how could be map lookup possibly faster than slice
indexing. Have you some measurements to share?

Same applies to using the switch as suggested by Vincent.

In both cases, the branch predictor and or speculative execution in
modern CPUs, can completely mislead measuring small cases.

[pierre...@gmail.com]

The const/slice implementation should be faster as Jan said.

Note that there is also a blank function that fails at compile time if items have been added or changed and stringer has not been rerun since.

The const/slice implementation allows for that (useful!) check.

[Brian Candler]

On Tuesday, 18 February 2020 09:04:49 UTC, Jan Mercl wrote:

> A simpler and faster way again would be to use a map.
> https://play.golang.org/p/ntjhesMsSA9

I don't see how could be map lookup possibly faster than slice
indexing. Have you some measurements to share?

No, I didn't measure it - I'll let the OP do that.  I still say it is simpler though.

[Vincent Blanchon]

@Brian, this code is generated by the stringer command based on this code: https://play.golang.org/p/JNfRC5TZYlo

@Jan, yes definitely. By the way, here are some numbers from a benchmark I made with 20 constants:

name                  time/op
Stringer-4             4.16ns ± 2%

StringerWithSwitch-4   3.81ns ± 1%
StringerWithMap-4     28.60ns ± 2%

Map is definitely the slowest one.

@Pierre, the compilation check can also be done if we do not have a slice. In my example with the switch, I still kept the compilation check. This check just use the constants, no relation with the "String()" function.

[Jan Mercl]

On Tue, Feb 18, 2020 at 10:37 AM Vincent Blanchon
<blanchon...@gmail.com> wrote:

> @Jan, yes definitely. By the way, here are some numbers from a benchmark I made with 20 constants:
>
> name time/op
> Stringer-4 4.16ns ± 2%
>
> StringerWithSwitch-4 3.81ns ± 1%
>
> StringerWithMap-4 28.60ns ± 2%

20 is a small number. In general, measuring things where one iteration
is in single digit nanoseconds can be misleading as discussed earlier.
The probably most important thing is how a chosen implementation
scales. To measure that, I suggest to make the number of the constants
in hundreds, if not thousands and having the loop exercise all the
paths, so the branch predictor cannot learn a single, happy one. In
any case, it's much safer to shift the time/op to at least
microseconds, otherwise the overhead of the test per se becomes
comparable to the measured thing and it just adds noise at minimum.

> @Pierre, the compilation check can also be done if we do not have a slice. In my example with the switch, I still kept the compilation check.

I can't find any compile time check in your code (the one using the
switch) that can detect a constant has changed its value since
stringer was run.

[Vincent Blanchon]

@Jan Thank you for the feedback. I will try with a higher number of constants. By the way, my benchmark tries all the path:

func BenchmarkStringerWithSwitch(b *testing.B) {
   for i := 0; i < b.N ; i++  {
      p := Pill(i%20)
      _ = p.String()
   }
}

 

I can't find any compile time check in your code 

 

I did not add it since it was not the original question ^^

But why can't we have the check and a switch?

Here is the code:

package main

import "strconv"

func _() {
   // An "invalid array index" compiler error signifies that the constant values have changed.
   // Re-run the stringer command to generate them again.
   var x [1]struct{}
   _ = x[Placebo-0]
   _ = x[Aspirin-1]
   _ = x[Ibuprofen-2]

}

func (i Pill) String() string {
   switch i {
   case 0: return "Placebo"
   case 1: return "Aspirin"
   case 2: return "Ibuprofen"
   case 3: return "Paracetamol"
   default: return "Pill(" + strconv.FormatInt(int64(i), 10) + ")"
   }
}

[Jan Mercl]

On Tue, Feb 18, 2020 at 11:10 AM Vincent Blanchon
<blanchon...@gmail.com> wrote:

> @Jan Thank you for the feedback. I will try with a higher number of constants. By the way, my benchmark tries all the path:
>
> func BenchmarkStringerWithSwitch(b *testing.B) {
> for i := 0; i < b.N ; i++ {
> p := Pill(i%20)
> _ = p.String()
> }
> }

That's IMO a very good approach. However, there's one potential trap.
The `_ = f()` thing may, or may not, get optimized away completely,
depending on the particular compiler and its version, if the compiler
can prove the called function is side effects free. The usual
workaround is, for example, to actually assign the returned value to a
package level variable _and_ read it afterwards. Like in

foo = p.String()
..

and later, just before returning

if foo == "" { // Assuming none of the string represenations are ""
b.Fatal()
}

[pierre...@gmail.com]

I did not add it since it was not the original question ^^

But why can't we have the check and a switch?

Definitely can. Just didnt see it. My response was somewhat tangential to your question, sry.

[Vincent Blanchon]

With 100 constants:

Stringer-4             4.96ns ± 0%

StringerWithSwitch-4   4.99ns ± 1%

StringerWithMap-4     30.40ns ± 0%

The gap between the switch and the current implement is much smaller. I guess it is due to the number of JMP instructions the code has to go through.

It confirms that the results before was accurate. I guess if I increase the number of constant, the current implementation will become faster.

However, in real code, I guess we will have that many constants, so it does not make really sense to increase the number.

It does not explain why this choice of design^^

The only thing I can see is the binary size... it is slightly bigger with the switch and 100 constants.

[Jan Mercl]

On Tue, Feb 18, 2020 at 1:47 PM Vincent Blanchon
<blanchon...@gmail.com> wrote:

> However, in real code, I guess we will have that many constants, so it does not make really sense to increase the number.

Design decisions may be driven by the desire to maximize the
performance in the average case or in the worst case scenario.

That's why Go has a regexp package that's slower in the average case
wrt PCRE, but it guarantees linear, not exponential, performance of
the worst case.

[Vincent Blanchon]

Yes, definitely, good point.

Both are them are good depending on the case actually.

[David Finkel]

 In this case there's a blog post explaining the decision.  Mostly focusing on memory savings. https://blog.golang.org/generate

There's no question the generated method is ugly. That's OK, though, because humans don't need to work on it; machine-generated code is often ugly. It's working hard to be efficient. All the names are smashed together into a single string, which saves memory (only one string header for all the names, even if there are zillions of them). Then an array, _Pill_index, maps from value to name by a simple, efficient technique. Note too that _Pill_index is an array (not a slice; one more header eliminated) of uint8, the smallest integer sufficient to span the space of values. If there were more values, or there were negatives ones, the generated type of _Pill_index might change to uint16 or int8: whatever works best.

(the blog then discusses decisions for bitfield enums as well)

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/69db18c0-31a4-4d52-94c6-a168ef815e0a%40googlegroups.com.