Change https://golang.org/cl/122466 mentions this issue: encoding/json: use method expressions in the scanner

Sample patch with numbers uploaded above. The benchstat is comparing the commit against its parent.

I'm assuming that I didn't do anything obviously wrong. I've read the changes twice, and all the tests pass, so I think it is correct.

My guess is that every time a method expression is stored in a function pointer a new lambda w/ capture is allocated. i.e:

s.step = s.stateBeginValue

is translated to:

s.step = func(b byte) { s.stateBeginValue(b) }

which would explain the huge increase in allocations

Yes, you are probably right. Though I couldn't replicate it with isolated benchmarks (global func variables vs method expression variables), so I wasn't sure if it was the cause.

Still, ignoring the details of the compiler, it would be a bit of a shame if method expressions were more expensive by design. I'd hope that either the slowness or allocations could be minimised.

I doubt there's a way to avoid allocations when doing what is essentially a variable capture. A method pointer is necessarily "fatter" than a straight-up function pointer has it includes an extra pointer to the instance, otherwise you're back to the old code where you have to manually provide the instance pointer at every call site.

One possible cleanup would be to make step a method, like:

func (s *Scanner) step(b byte) {
  s.stepFunction(s, b)
}

which should presumably be inlined, avoiding any slowdown, and allow all call sites to be cleaner. It's not clear to me that it's worthwhile though...

I thought about the wrapper method :) Didn't try it because, without mid-stack inlining, it's likely to make things worse right now.

I don't think there's anything we can do here, and noone had any ideas either. Closing.