Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/link: support for zstd-compressed DWARF sections #55107

Open
stapelberg opened this issue Sep 16, 2022 · 27 comments
Open

cmd/link: support for zstd-compressed DWARF sections #55107

stapelberg opened this issue Sep 16, 2022 · 27 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@stapelberg
Copy link
Contributor

I recently read a blog post by @MaskRay comparing different compression algorithms for DWARF: https://maskray.me/blog/2022-09-09-zstd-compressed-debug-sections

The blog post concludes that zstd is the best choice (as it is in many other scenarios), and the author has done some work on establishing ELFCOMPRESS_ZSTD.

I don’t know if it’s too early, or if anything else is preventing us from doing so, but wanted to file this issue to track support for using zstd to make Go DWARF sections smaller and faster to write and read.

There is a good native Go implementation of zstd available: https://pkg.go.dev/github.com/klauspost/compress/zstd

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Sep 16, 2022
@dsnet
Copy link
Member

dsnet commented Sep 16, 2022

Would it make sense to consider a standard library "compress/zstd" package first before considering this?

@stapelberg
Copy link
Contributor Author

Would it make sense to consider a standard library "encoding/zstd" package first before considering this?

I don’t think we need to gate landing zstd DWARF on having a zstd package in the standard library — we could just import it into internal, or vendor it, or whatever is the currently preferred approach :)

But, having encoding/zstd in the standard library would certainly be a welcome addition IMHO :)

Can you open a separate issue for that please?

@MaskRay
Copy link

MaskRay commented Sep 16, 2022

@mengzhuo added ELFCOMPRESS_ZLIB support to cmd/link and may be interested:)

@Foxboron
Copy link
Contributor

I'd like to just point out that elfutils without support for ZSTD is going to be painfull for downstream as we can't make debug packages out of it. I'd rather have us disable compression and DWARF headers by default.

@stapelberg
Copy link
Contributor Author

Sounds like it should be behind a flag for now, or added only once elfutils support for zstd is more widespread.

@cherrymui
Copy link
Member

Do the debuggers support this format? GDB? LLDB? Delve? Thanks.

cc @aarzilli @thanm

@dsnet
Copy link
Member

dsnet commented Sep 16, 2022

Does the runtime ever look at the DWARF section, will we need to link in a zstd decompressor?

@cherrymui
Copy link
Member

@dsnet the runtime doesn't look at the DWARF sections.

@mengzhuo
Copy link
Contributor

@mengzhuo added ELFCOMPRESS_ZLIB support to cmd/link and may be interested:)

I'm glad to do that :) but as @dsnet suggests it looks like we need a encoding/zstd in the first place.

cc @rsc

@aarzilli
Copy link
Contributor

aarzilli commented Sep 17, 2022

Do the debuggers support this format? GDB? LLDB? Delve? Thanks.

cc @aarzilli @thanm

According to the blog post, it was proposed to SysV gABI in june of this year and it is supported by approximately nothing. Delve will support it when debug/elf will support it, so I think it should be added there first (or at least at the same time). Other consideration:

  • I think @dsnet is right that there should be a compress/zstd first, it would be weird if the standard library supported decompressing zstd but only if it's inside an elf container
  • this is all about elf compressed sections, it does not apply to zdebug sections, which are emitted for mach-o and PE, and will need to stay on zlib
  • given how slow binutils moves in LTS distributions this likely can not be the default (or only) option for several years
  • the list of feature requests at the end of the article is highly incomplete, for example bloaty (which the article even uses) will have to be changed to support zstd compressed elf sections
  • it probably won't be a problem but compression speed is also a factor for cmd/link.

@cherrymui cherrymui added this to the Unplanned milestone Sep 19, 2022
@cherrymui cherrymui added FeatureRequest help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 19, 2022
@MaskRay
Copy link

MaskRay commented Sep 27, 2022

FWIW clang/ld.lld/llvm-objcopy has supported zstd for ~2 weeks now. I just landed zstd support to binutils and gdb https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=2cac01e3ffff74898c54fa5e6418817f5578adb6 :)

@znkr
Copy link
Contributor

znkr commented Oct 17, 2022

IIUC, cgo is reading object files created by a C compiler, using debug/elf. Clang using zstd for compression is going to cause an incompatibility should it switch to zstd by default.

@Foxboron
Copy link
Contributor

The default is controlled by a compile-time flag which should be selected by the distro, along with dwarf compression being off by default. I don't think it's too much to worry about yet?

@znkr
Copy link
Contributor

znkr commented Oct 17, 2022

I agree, I just wanted to make sure to mention that Go also consumes object files as another reason to support zstd. :)

algitbot pushed a commit to alpinelinux/aports that referenced this issue Jan 9, 2023
closes #14537

perhaps solved by golang/go#55107 eventually, adding the same support to go

golang.... google.....
@marxin
Copy link

marxin commented Feb 22, 2023

I would like to point out that the latest binutils release 2.40 supports the zstd compression. Similarly master branch of elfutils (waiting for a release). So please consider adding support for cgo soon.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/473356 mentions this issue: internal/zstd: new internal package for zstd decompression

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/473256 mentions this issue: debug/elf: support zstd compression

@ianlancetaylor ianlancetaylor self-assigned this Mar 6, 2023
@ianlancetaylor ianlancetaylor modified the milestones: Unplanned, Backlog Mar 6, 2023
@mknyszek mknyszek moved this to In Progress in Go Compiler / Runtime Mar 8, 2023
bell-sw pushed a commit to bell-sw/alpaquita-aports that referenced this issue Mar 15, 2023
[ commit 01358ccb6df70c5fc1cc7ed451a19a75eb419ad9 ]

closes #14537

perhaps solved by golang/go#55107 eventually, adding the same support to go

golang.... google.....
gopherbot pushed a commit that referenced this issue Apr 18, 2023
This package only does zstd decompression, which is starting to
be used for ELF debug sections. If we need zstd compression we
should use github.com/klauspost/compress/zstd. But for now that
is a very large package to vendor into the standard library.

For #55107

Change-Id: I60ede735357d491be653477ed419cf5f2f0d3f71
Reviewed-on: https://go-review.googlesource.com/c/go/+/473356
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Compiler / Runtime Apr 18, 2023
@znkr
Copy link
Contributor

znkr commented Apr 19, 2023

IIUC, zstd is not used for -compressdwarf yet and thus can't be used to compress DWARF sections in Go binaries. Was the issue intentionally closed without adding support in cmd/link?

@ianlancetaylor
Copy link
Member

Sorry, I think I misunderstood this issue. The immediate problem of handling code generated by newer tools is addressed. But you're right that cmd/link still does not compress DWARF with zstd. That is a bigger hill to climb. With our current approach to vendoring in the standard library, I don't know that we want to vendor in a large package like github.com/klauspost/compress/zstd, which includes assembly code. Anyhow, reopening.

@github-project-automation github-project-automation bot moved this from Done to In Progress in Go Compiler / Runtime Apr 19, 2023
@znkr
Copy link
Contributor

znkr commented Apr 19, 2023

I think the issue was conflating two problems. Support for zstd when compiling Go and interop with C/C++ toolchains using zstd. The latter should be fixed now. Thank you. :)

For supporting zstd in Go itself: It might also be interesting to compress dwarf sections in object files as well and not just in the linker. If I find the time I might hack something together to get some data.

@cherrymui
Copy link
Member

For supporting zstd in Go itself: It might also be interesting to compress dwarf sections in object files as well and not just in the linker.

The Go object file doesn't have DWARF sections. It just has a bunch of symbols, some of which may be discarded at link time. And there are some DWARF data generated at link time. Maybe I missed something, but I'm not sure we want to do DWARF compression in object files.

Also, how is this related to zstd? Whether to compress in object files or at link time, and which compression algorithm to use look pretty orthogonal to me. Or zstd has anything special that prefers one or the other?

@znkr
Copy link
Contributor

znkr commented Apr 19, 2023

My bad. I don’t really understand the whole process very well. What I am wondering, and this should likely be a new issue, is if it’s possible to compress debug sections in files similar to clang’s -gz flag. zstd is interesting here because it provides the right tradeoffs between compression speed and size and that could improve build speeds especially with larger builds: https://maskray.me/blog/2022-09-09-zstd-compressed-debug-sections

@cherrymui
Copy link
Member

Thanks. By "compress dwarf sections in object files" maybe you mean in external linking mode (pre-)compressing the DWARF sections in the go.o file that the Go linker passes to the C linker? Maybe we could do that. We probably could also do that for other compression algorithms. Not sure if there is anything zstd specific.

@ianlancetaylor
Copy link
Member

ianlancetaylor commented Apr 19, 2023

Just to be clear, in internal linking mode cmd/link already compresses the DWARF sections using zlib. So one aspect of this issue would be to compress them using zstd.

@znkr
Copy link
Contributor

znkr commented Apr 20, 2023

I think where I got things wrong is assuming that Go object files have the same format as clang and GCC formats. IIUC, that's not the case and Go has a custom format. @cherrymui, from your comment, I am guessing that there is no explicit debug info section in Go object files that could be compressed. If that's the case, my previous comment about compressing DWARF sections in Go object files is based on me assuming that there is such a thing in Go, because it exists in C.

That said, compressing the debug information in object files for external linking might be interesting. I'll see if I can get some data on that.

@znkr
Copy link
Contributor

znkr commented Apr 20, 2023

I performed some measurements by using objcopy --compress-debug-sections=zstd on intermediate object files. The linking speed of the external linker was about the same, but the file sizes shrank quite significantly. In one case from 3.2 GiB to 2.2 GiB.

@ianlancetaylor ianlancetaylor removed their assignment Apr 21, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/499616 mentions this issue: doc/go1.21: mention debug/elf changes

gopherbot pushed a commit that referenced this issue May 31, 2023
For #55107
For #56887
For #56892

Change-Id: Ibcca34c931ed3595d877c4573bba221dec66bb5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/499616
Reviewed-by: Eli Bendersky <eliben@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Status: In Progress
Development

No branches or pull requests