go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
44.75k stars 5.47k forks source link

[Proposal] Use Golang 1.18 for Gitea 1.17 release #19917

Closed wxiaoguang closed 2 years ago

wxiaoguang commented 2 years ago

Use Golang 1.18 (as minimal requirement) for Gitea 1.17 release, make sure the Golang version is still actively supported during Gitea 1.17 lifecycle.

And Gitea can start using Golang generics from then on.

zeripath commented 2 years ago

Not sure I think that we should immediately jump on to Generics before releasing 1.17 but I agree with making Gitea 1.17 use Golang 1.18.


One thing against moving to generics already is that a lot of the std library remains ungenercised and certainly most of our downstream libraries. This situation will improve over time and whilst I know someone has to start, the wrong steps now could lead to annoying refactors later.

Further:

https://planetscale.com/blog/generics-can-make-your-go-code-slower

Whilst Gitea code is frankly far from optimised enough to make most of these performance losses a serious problem, this comment is worth noticing as it would be relevant to a lot of string manipulations:

The results are not surprising. The function that has been specialized to take a *strings.Builder directly is the fastest, because it allowed the compiler to inline the WriteByte calls inside of it. The generic function is measurably slower than the simplest possible implementation taking an io.ByteWriter interface as an argument. We can see that the impact of the extra load from the generic dictionary is not significant, because both the itab and the Generics dictionary will be very warm in cache in this micro-benchmark (however, do keep reading for an analysis of how cache contention affects Generic code).

This is the first insight we can gather from this analysis: there’s no incentive to convert a pure function that takes an interface to use Generics in 1.18. It’ll only make it slower, as the Go compiler cannot currently generate a function shape where methods are called through a pointer. Instead, it will introduce an interface call with two layers of indirection. This is going in the exact opposite direction of what we’d like, which is de-virtualization and, where possible, inlining.

Although genericising the WriteByte/WriteString to WriteByteSeq() would likely be beneficial.

The Summary here is enlightening:

This was a lot of fun! I hope you had a lot of fun too looking at these assemblies with me. Let’s finish this post with a short list of DOs and DON’Ts when it comes to performance and Generics in Go 1.18:

  • DO try to de-duplicate identical methods that take a string and a []byte using a ByteSeq constraint. The generated shape instantiation is very close to manually writing two almost-identical functions.
  • DO use generics in data structures. This is by far their best use case: Generic data structures that were previously implemented using interface{} are complex and un-ergonomic. Removing the type assertions and storing types unboxed in a type-safe way makes these data structures both easier to use and more performant.
  • DO attempt to parametrize functional helpers by their callback types. In some cases, it may allow the Go compiler to flatten them.
  • DO NOT attempt to use Generics to de-virtualize or inline method calls. It doesn’t work because there’s a single shape for all pointer types that can be passed to the generic function; the associated method information lives in a runtime dictionary.
  • DO NOT pass an interface to a generic function, under any circumstances. Because of the way shape instantiation works for interfaces, instead of de-virtualizing, you’re adding another virtualization layer that involves a global hash table lookup for every method call. When dealing with Generics in a performance-sensitive context, use only pointers instead of interfaces.
  • DO NOT rewrite interface-based APIs to use Generics. Given the current constraints of the implementation, any code that currently uses non-empty interfaces will behave more predictably, and will be simpler, if it continues using interfaces. When it comes to method calls, Generics devolve pointers into twice-indirect interfaces, and interfaces into… well, something quite horrifying, if I’m being honest.
  • DO NOT despair and/or weep profusely, as there is no technical limitation in the language design for Go Generics that prevents an (eventual) implementation that uses monomorphization more aggressively to inline or de-virtualize method calls.

I do think that the go runtime and compiler will likely have further improvements in 1.19 etc with regards to how it handles generics internally. As would the std library and many of our upstream libraries.


Do you have a serious place where generics would be appropriate?


I think instead we should be looking at what's blocking release of 1.17-RC1 - and if there's nothing that we think is really missing or we need in 1.17 we should just cut it and then we can get started on 1.18, allow generics and do more major refactorings.

If we make Gitea 1.17 only support go 1.18 then if there are generics changes that need to be backported we can do so - but I think adding generics at this point is too late. It's just going to further delay 1.17 - which we need to get ready for release.

wxiaoguang commented 2 years ago

Do you have a serious place where generics would be appropriate?

Nope at the moment.


Two things here:

  1. Make Gitea use Golang 1.18 for next release
  2. Make Gitea start using Golang Generics

For question 1, I think it's reasonable because Golang's release cycle is very fast, a few month later, Golang 1.19 comes and Golang 1.17 will be unsupported. So, making Gitea use an active Golang 1.18 should be good.

For question 2, I think we can wait and see, some code could benefit from it, for example, container.KeysInt64, maybe more. I also agree that it's not a must, just fine to have. So the decision doesn't need to be made too soon.