go icon indicating copy to clipboard operation
go copied to clipboard

Getting rid of reflect2 dependency/package

Open advancedwebdeveloper opened this issue 4 years ago • 14 comments

Hi. People are experiencing compilation issues, with gollvm, when compiling Kubernetes.

The reason is in reflect2 - it a dependency of this project, in turn being a dependency of Kubernetes.

It was pretty shocking to admit that three different implementations of reflection related package exist:

https://golang.org/pkg/reflect/ Previously mentioned reflect2 https://github.com/goccy/go-reflect . Reflection is indeed a core feature for many programming languages. As for the fact that json-iterator is depending on no-more-maintained sub-package - that smells like the current system design/architecture of Kubernetes, which doesn't change frequently (and farther re-design is pretty predictable) , depends on json-iterator itself only at the current stage. I have nothing against this project - but it makes sense to split API implementations into (at least) two different Golang packages, each of which wouldn't have no common sub-dependencies/packages.

I have pinged key committers of k8s - and they pointed me into those links:

Reduce sizes of binaries #76506 Use json-iterator instead of ugorji for JSON. #48287

As for the practical side of the story - here is a list on already opened bugs:

undefined reference to 'reflect.typelinks' for gccgo and gollvm Build kubernetes with gollvm bugs with gollvm

Thanks to @mrVanboy - he pointed me into https://github.com/json-iterator/go/blob/master/reflect.go#L40-L41 .

advancedwebdeveloper avatar Oct 14 '20 11:10 advancedwebdeveloper

There are much more implementations of reflect libraries: https://libs.garden/go/search?q=reflect

I think, that the non-stdlib reflect library has been chosen primarily because of better performance.

One of the solutions which cross my mind is introducing an interface for the reflection dependency and allowing a user to decide (e.g. via options or build tags) which implementation them prefers.

This approach also allows us to write benchmarks and compile different reflect packages and how they are affecting performance. I don't think that replacing reflect2 package blindly without benchmarks is great.

mrVanboy avatar Oct 14 '20 19:10 mrVanboy

Hi, I'm the author of https://github.com/goccy/go-reflect . Also, I'm the author of https://github.com/goccy/go-json that I develop using the goccy/go-reflect technique. goccy/go-json is much faster than the json-iterator .

github.com/goccy/go-reflect call the standard reflect library without allocation. By using this library for the type determination part of the json-iterator, you can encode the value passed at Marshal without escaping it. This eliminates unnecessary allocations and is faster.


I don't think it's a good idea to keep using an unmaintained library . But if you're going to replace the library, I also think it's a good idea to get a benchmark along with it. I'm sure you'll get good results.

Also, although this isn't really what I should be writing about here, the github.com/goccy/go-json has the same concept as json-iterator/go , but it's so faster. I would be happy to refer to it in conjunction with goccy/go-reflect .

goccy avatar Oct 15 '20 08:10 goccy

It was pretty shocking to admit that three different implementations of reflection related package exist:

https://golang.org/pkg/reflect/ CC @paleozogt @heylinn

advancedwebdeveloper avatar Oct 15 '20 13:10 advancedwebdeveloper

CC @fedebongio @deads2k

advancedwebdeveloper avatar Oct 17 '20 13:10 advancedwebdeveloper

Hi, I'm the author of https://github.com/goccy/go-reflect . Also, I'm the author of https://github.com/goccy/go-json that I develop using the goccy/go-reflect technique. goccy/go-json is much faster than the json-iterator .

github.com/goccy/go-reflect call the standard reflect library without allocation. By using this library for the type determination part of the json-iterator, you can encode the value passed at Marshal without escaping it. This eliminates unnecessary allocations and is faster.

I don't think it's a good idea to keep using an unmaintained library . But if you're going to replace the library, I also think it's a good idea to get a benchmark along with it. I'm sure you'll get good results.

Also, although this isn't really what I should be writing about here, the github.com/goccy/go-json has the same concept as json-iterator/go , but it's so faster. I would be happy to refer to it in conjunction with goccy/go-reflect .

@Fenny , would you suggest specific benchmarking scenarios?

advancedwebdeveloper avatar Oct 17 '20 13:10 advancedwebdeveloper

I don't think compatibility with a non-standard go compiler is a good reason to make a significant change here. If there are other reasons, like better performance or closer adherence to the stdlib reflect package that fix functional bugs, that could be a reasonable motivation.

Has the incompatibility with the standard go compiler been reported to gollvm?

liggitt avatar Oct 18 '20 14:10 liggitt

I don't think compatibility with a non-standard go compiler is a good reason to make a significant change here. If there are other reasons, like better performance This is where gollvm could show some advantage, by using the flexibility of LLVM's back-ends, for specific arch.

or closer adherence to the stdlib reflect package

Please provide some examples - I can't catch/follow your thought precisely.

that fix functional bugs, that could be a reasonable motivation. OK

Has the incompatibility with the standard go compiler been reported to gollvm?

If you would be attentive to the reports - you would already mention that is was. @ianlancetaylor , please provide your vision for the common Go language front-end.

advancedwebdeveloper avatar Oct 18 '20 14:10 advancedwebdeveloper

@ianlancetaylor , please provide your vision for the common Go language front-end.

I can't force all package maintainers to support gccgo or GoLLVM. I think it's unfortunate that people choose to run unsafe code, but ultimately that is their prerogative.

ianlancetaylor avatar Oct 22 '20 23:10 ianlancetaylor

@ianlancetaylor , I am curious about your opinion on mentioned reflection libraries

advancedwebdeveloper avatar Oct 24 '20 13:10 advancedwebdeveloper

I think it's risky to use them, as they are prone to break with each new Go release. I think it would be better to file bugs against the standard reflect package to look for standard ways to address whatever problems they are seeing. I don't know enough about them to know why they exist.

ianlancetaylor avatar Oct 24 '20 17:10 ianlancetaylor

@goccy, please explain your motivation (why did you decided to create an alternative reflection package; which issues where you facing, for the standard reflection package)

advancedwebdeveloper avatar Oct 24 '20 21:10 advancedwebdeveloper

https://github.com/json-iterator/go/issues/462 - another cause for refactoring

advancedwebdeveloper avatar Nov 07 '20 20:11 advancedwebdeveloper

So, I tired to experiment on Windows 10 (and ordinary Go compiler) - just to make sure it would be well tested, before porting for gollvm, on Linux.

https://gist.github.com/advancedwebdeveloper/eeba866732258f34cfb45cf68274d6c1 - here is what I did.

Here is what I got:

$ go build -i -v -x WORK=C:\Users\Worker\AppData\Local\Temp\go-build473407762

get https://proxy.golang.org/github.com/goccy/go-reflect/@v/v0.0.0-20180701023420-4b7aa43c6742.mod

get https://proxy.golang.org/github.com/goccy/go-reflect/@v/v0.0.0-20180701023420-4b7aa43c6742.mod: 410 Gone (2.461s)

mkdir -p C:\Users\Worker\go\pkg\mod\cache\vcs # git3 https://github.com/goccy/go-reflect

lock C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f.lockmkdir -p C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f # git3 https://github.com/goccy/go-reflect

cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git init --bare 0.107s # cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git init --bare cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git remote add origin -- https://github.com/goccy/go-reflect 0.067s # cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git remote add origin -- https://github.com/goccy/go-reflect cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git -c log.showsignature=false log -n1 '--format=format:%H %ct %D' 4b7aa43c6742 -- 0.054s # cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git -c log.showsignature=false log -n1 '--format=format:%H %ct %D' 4b7aa43c6742 -- cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git tag -l 0.052s # cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git tag -l cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git ls-remote -q origin 0.626s # cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git ls-remote -q origin cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git fetch -f origin 'refs/heads/:refs/heads/' 'refs/tags/:refs/tags/' 1.462s # cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git fetch -f origin 'refs/heads/:refs/heads/' 'refs/tags/:refs/tags/' cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git -c log.showsignature=false log -n1 '--format=format:%H %ct %D' 4b7aa43c6742 -- 0.137s # cd C:\Users\Worker\go\pkg\mod\cache\vcs\249863f81ac2c294c1b35708f840ef5ec342fe365f439a3eba7bde2ed1ab3a8f; git -c log.showsignature=false log -n1 '--format=format:%H %ct %D' 4b7aa43c6742 -- go: github.com/goccy/[email protected]: invalid version: unknown revision 4b7aa43c6742

I am using

$ go version go version go1.15.8 windows/amd64 . Also hence

$ go env set GO111MODULE= set GOARCH=amd64 set GOBIN= set GOCACHE=C:\Users\Worker\AppData\Local\go-build set GOENV=C:\Users\Worker\AppData\Roaming\go\env set GOEXE=.exe set GOFLAGS= set GOHOSTARCH=amd64 set GOHOSTOS=windows set GOINSECURE= set GOMODCACHE=C:\Users\Worker\go\pkg\mod set GONOPROXY= set GONOSUMDB= set GOOS=windows set GOPATH=C:\Users\Worker\go set GOPRIVATE= set GOPROXY=https://proxy.golang.org,direct set GOROOT=C:\Program Files\Go set GOSUMDB=sum.golang.org set GOTMPDIR= set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64 set GCCGO=gccgo set AR=ar set CC=gcc set CXX=g++ set CGO_ENABLED=1 set GOMOD=C:\Users\Worker\Desktop\json_goccy\go.mod set CGO_CFLAGS=-g -O2 set CGO_CPPFLAGS= set CGO_CXXFLAGS=-g -O2 set CGO_FFLAGS=-g -O2 set CGO_LDFLAGS=-g -O2 set PKG_CONFIG=pkg-config set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\Worker\AppData\Local\Temp\go-build607521154=/tmp/go-build -gno-record-gcc-switches

advancedwebdeveloper avatar Feb 09 '21 16:02 advancedwebdeveloper

I tried to do like here https://gist.github.com/advancedwebdeveloper/e05d9fd1f277d0584456b4aaf4ba2983 - and here is what I got:

$ go build -i -v -x WORK=/tmp/go-build283270276 github.com/json-iterator/go mkdir -p $WORK/b001/ cat >$WORK/b001/importcfg << 'EOF' # internal

import config

packagefile bytes=/usr/local/pkg/linux_386/bytes.a packagefile encoding=/usr/local/pkg/linux_386/encoding.a packagefile encoding/base64=/usr/local/pkg/linux_386/encoding/base64.a packagefile encoding/json=/usr/local/pkg/linux_386/encoding/json.a packagefile errors=/usr/local/pkg/linux_386/errors.a packagefile fmt=/usr/local/pkg/linux_386/fmt.a packagefile github.com/goccy/go-reflect=/home/oceanfish81/.cache/go-build/64/64ae6739902a6fc8353cc14e5d314c8713174e26c80340137d026c327a0ecb54-d packagefile github.com/modern-go/concurrent=/home/oceanfish81/.cache/go-build/3d/3d3f503f5a323c5295a05087fbcf5091f4ad577887e10811ea149e5610cba742-d packagefile io=/usr/local/pkg/linux_386/io.a packagefile math=/usr/local/pkg/linux_386/math.a packagefile math/big=/usr/local/pkg/linux_386/math/big.a packagefile reflect=/usr/local/pkg/linux_386/reflect.a packagefile sort=/usr/local/pkg/linux_386/sort.a packagefile strconv=/usr/local/pkg/linux_386/strconv.a packagefile strings=/usr/local/pkg/linux_386/strings.a packagefile sync=/usr/local/pkg/linux_386/sync.a packagefile unicode=/usr/local/pkg/linux_386/unicode.a packagefile unicode/utf16=/usr/local/pkg/linux_386/unicode/utf16.a packagefile unicode/utf8=/usr/local/pkg/linux_386/unicode/utf8.a EOF cd /home/oceanfish81/Desktop/json_iterator_goccy /usr/local/pkg/tool/linux_386/compile -o $WORK/b001/pkg.a -trimpath "$WORK/b001=>" -p github.com/json-iterator/go -lang=go1.12 -complete -buildid AWqWdwzwFLyGPNvqPz88/AWqWdwzwFLyGPNvqPz88 -goversion go1.15.8 -D "" -importcfg $WORK/b001/importcfg -pack ./adapter.go ./any.go ./any_array.go ./any_bool.go ./any_float.go ./any_int32.go ./any_int64.go ./any_invalid.go ./any_nil.go ./any_number.go ./any_object.go ./any_str.go ./any_uint32.go ./any_uint64.go ./config.go ./iter.go ./iter_array.go ./iter_float.go ./iter_int.go ./iter_object.go ./iter_skip.go ./iter_skip_strict.go ./iter_str.go ./jsoniter.go ./pool.go ./reflect.go ./reflect_array.go ./reflect_dynamic.go ./reflect_extension.go ./reflect_json_number.go ./reflect_json_raw_message.go ./reflect_map.go ./reflect_marshaler.go ./reflect_native.go ./reflect_optional.go ./reflect_slice.go ./reflect_struct_decoder.go ./reflect_struct_encoder.go ./stream.go ./stream_float.go ./stream_int.go ./stream_str.go

github.com/json-iterator/go

./any.go:259:15: undefined: "github.com/goccy/go-reflect".TypeOfPtr ./reflect_array.go:36:15: undefined: "github.com/goccy/go-reflect".UnsafeArrayType ./reflect_array.go:60:15: undefined: "github.com/goccy/go-reflect".UnsafeArrayType ./reflect_dynamic.go:55:11: undefined: "github.com/goccy/go-reflect".UnsafeIFaceType ./reflect_map.go:142:15: undefined: "github.com/goccy/go-reflect".UnsafeMapType ./reflect_map.go:245:15: undefined: "github.com/goccy/go-reflect".UnsafeMapType ./reflect_map.go:279:15: undefined: "github.com/goccy/go-reflect".UnsafeMapType ./reflect_native.go:407:16: undefined: "github.com/goccy/go-reflect".UnsafeSliceType ./reflect_slice.go:23:15: undefined: "github.com/goccy/go-reflect".UnsafeSliceType ./reflect_slice.go:55:15: undefined: "github.com/goccy/go-reflect".UnsafeSliceType ./any.go:259:15: too many errors

I was using

$ go version go version go1.15.8 linux/386

$ go env GO111MODULE="" GOARCH="386" GOBIN="" GOCACHE="/home/oceanfish81/.cache/go-build" GOENV="/home/oceanfish81/.config/go/env" GOEXE="" GOFLAGS="" GOHOSTARCH="386" GOHOSTOS="linux" GOINSECURE="" GOMODCACHE="/home/oceanfish81/go/pkg/mod" GONOPROXY="" GONOSUMDB="" GOOS="linux" GOPATH="/home/oceanfish81/go" GOPRIVATE="" GOPROXY="https://proxy.golang.org,direct" GOROOT="/usr/local" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/usr/local/pkg/tool/linux_386" GCCGO="gccgo" GO386="sse2" AR="ar" CC="gcc" CXX="g++" CGO_ENABLED="1" GOMOD="/home/oceanfish81/Desktop/json_iterator_goccy/go.mod" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -m32 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build149488018=/tmp/go-build -gno-record-gcc-switches"

Apparently an API is compatible

advancedwebdeveloper avatar Feb 10 '21 01:02 advancedwebdeveloper