Minor bot API methods call optimizations.

[kvizyx]

While researching telego, I found some "free" optimizations that don't change the library API and don't affect code readability (if not improve it) related to the pointers management in bot API methods and general Telegram API calling function. They reduce a couple of allocations when calling any bot method, and increase its speed a bit (about ~10% depending on the method's payload).

goos: linux
goarch: amd64
pkg: ptr-bench
cpu: Intel(R) Core(TM) i5-9400F CPU @ 2.90GHz
BenchmarkCallBotMethodWithPayloadOld-6           1722831               667.3 ns/op           264 B/op          8 allocs/op
BenchmarkCallBotMethodWithPayloadNew-6           2108314               562.3 ns/op           240 B/op          6 allocs/op

Do you accept (or it's better to say does telego need) such changes, or there is no point in me doing it?

[mymmrac]

Hi @kvizyx , thanks for the research, feel free to submit a PR, I will review it, if it doesn't break anything, I will be happy to merge that

If this change is big, you can show a simple example for one method and I will first review that and we can continue from that point

[kvizyx]

Basically this optimization boils down to 2 changes:

1. Remove pointer to vs here and use loop value instead.
2. Remove pointer to the request output (example) and return reference to it at the end instead (for all methods that have some output).

That's why I called this optimizations "free" :)

I also don't understand why telego use pointers for methods params and return values.

[mymmrac]

Hmm, interesting, I will look into it, thanks

Regarding pointers for parameters - two main reasons, first is to no copy values when passing them to functions, second is to be able to have .With... methods on parameters

Regarding return values - no strong reason for that, but one is that it's consistent as some methods return interfaces

[kvizyx]

The only problem I didn't realize at first is that the telego methods generator sets some return values as optional even though they aren't actually, so it's hard to distinguish them from the truly optional ones. From my perspective, there are 2 solutions:

1. Make all return values not optional and generate additional "null" values for each type (for custom structs) and add check for null value in each bot method like this:

// Generated types
type (
	Result struct {
		Value string `json:"value"`
	}

	resultNil = Result{}
)

// Generated method
func (b *Bot) CallSomeBotAPIMethod() (Result, error) {
	var result Result
	err := b.performRequest(ctx, "someBotAPIMethod", nil, &result)
	if err != nil {
		return nil, fmt.Errorf("telego: callSomeBotAPIMethod: %w", err)
	}
	if result == resultNil {
		return nil, nil
	}
	return &result, nil
}

2. Just make all return values not optional, but it will break API as someone might check return value for nil.

So I think this optimizations doesn't make sense, as we have to do too much for a small performance improvement that most won't even notice if its not a high-loaded bot.

[mymmrac]

Yep, some methods can return optional results, but all can have an error => result is kind of always optional, but in some cases even if there is no error you must check for nil