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/compile: consider adding sibling links to generated DWARF #22185

Open
thanm opened this issue Oct 9, 2017 · 5 comments
Open

cmd/compile: consider adding sibling links to generated DWARF #22185

thanm opened this issue Oct 9, 2017 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Debugging NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@thanm
Copy link
Contributor

thanm commented Oct 9, 2017

The DWARF .debug_info section generated by the current Go compiler
does not make use of sibling links at the moment. As gc-generated
DWARF DIE's begin to get more substantial/complicated, this will make
it more time-consuming for debugger as it tries to locate the DIE for
a specific entity within the .debug_info section.

It might make sense to look into introducing siblings to DIEs such as
the subprogram DIE and possibly the structure type DIE (for use in cases
where you have a struct with a large number of members).

Note that such a change would provide little benefit for Delve users
without a corresponding change to Delve (looking at the Delve tip
source code, it appears that the DWARF reader is not set up to exploit
sibling links).

@aarzilli @heschik

@heschi
Copy link
Contributor

heschi commented Oct 9, 2017

I know at least 3 ways of getting around DWARF: sibling links, .debug_pubtypes, .debug_names (DWARF5) and I think there are some hash table variants floating around. Do you have any context on which is the most bestest? We already have .pubtypes and .pubnames, the latter of which should cover finding functions.

@mdempsky
Copy link
Member

mdempsky commented Oct 9, 2017

I would think the indices (debug_pubtypes or debug_names) are better than providing sibling links?

Debug_names seems slightly better than debug_pubtypes/debug_pubnames because users only need to look up in one index instead of two, but it's also newer so it's presumably less widely supported yet.

@thanm
Copy link
Contributor Author

thanm commented Oct 9, 2017

Looking at the source for Delve, I see code that walks the entire .debug_info section, looking for interesting bits and the calling "reader.SkipChildren()" in situations where the children are not interesting. Example:

https://github.com/derekparker/delve/blob/5c9b2009cab214c1f912a2d8c6c89eec9f3ada2b/pkg/proc/types.go#L142

If we added sibling links to the GC-generated DWARF, I'd imagine that this routine would get a lot faster (without any changes to the code itself, I should add).

@thanm
Copy link
Contributor Author

thanm commented Oct 9, 2017

I should add to my last comment-- my original remark about needing a corresponding change in Delve to exploit sibling links was incorrect; delve is getting that for free by using Go's debug/dwarf package.

@aarzilli
Copy link
Contributor

Note that routine is only executed once on startup. And #19340 is a far bigger contributor to slow start up times at the moment.

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 29, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 29, 2018
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
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. Debugging NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants