- User Since
- Aug 17 2020, 8:52 AM (68 w, 10 h)
Aug 2 2021
Thanks for the quick review! Please commit it as I do not have permission to do that.
Jul 30 2021
Is arm hardware necessary to test this, or can the test be modified to cross-compile to arm to see what is going on? Is there a way to determine what build target the test bot is using?
Would it make sense to turn the split-optimized test back into an x86 only test, or just leave it out of the change as it's not actually testing a code path that this changed?
Jul 29 2021
Restoring previous diff.
Argh. Okay, I need to close this tab.
Jan, let me know if there's anything else I should change. Also, I believe I will need someone else to actually submit this for me.
Jul 28 2021
The only changed behavior is CompileUnit::Dump(), which I've changed to distinguish between an uncalculated languange ("<not loaded>") and an unspecified one ("unknown"). No other API should behave differently, as GetLanguage() will trigger parsing of the non-skeleton unit, and no API would be parsing language information out of Dump() output. With my latest update there is also no effect if not using -gsplit-dwarf, and this change substantially benefits those using -gsplit-dwarf. In practice I don't see a compelling user story where this would be a regression.
Jan, I don't think there's any point in checking for the case when the dwo is already loaded as that wouldn't be triggered if the CompileUnit hasn't already been created. However I added a check for the case where there is no dwo because it wasn't compiled with -gsplit-dwarf. Note that the debug-types-address-ranges.s test case no longer requires modification in this version. Let me know if you have any further nitpicks for SymbolFileDWARF::ParseCompileUnit. Nothing else required changes since your last review. I'm also happy to bring back the previous version with your nitpicks fixed if you'd prefer.
Now eagerly evaluates the language if there is no dwo. Lazy evaluation only happens for split dwarf case.
Jul 27 2021
Jul 26 2021
The "<not loaded>" message would never be seen without the changes in this patch, so I don't think it makes sense to do independently.
Yes I did, thank you. This patch will probably be abandoned anyway if https://reviews.llvm.org/D106355 is accepted.
Now distinguishing between unknown language and not loaded.
Language in Dump now distinguishes between unknown and not loaded.
GetLanguage has now a bug for -gsplit-dwarf as it will return eLanguageTypeUnknown if called on the skeleton - which could affect ManualDWARFIndex::IndexUnitImpl but then it is using LanguageType cu_language only for cu_language == eLanguageTypeObjC. Which is needed only on OSX and OSX does not have -gsplit-dwarf. GetLanguage should IMO force loading of the DWO.
So dump always passes a the skeleton unit to "GetLanguage" or it already makes a dynamic choice based on whether or not the dwo has already been loaded?
Jul 23 2021
Updated and hopefully a little simpler now.
Jul 21 2021
Thanks for your feedback. I was off for a long weekend but will finish fixing up this change tomorrow.
Thank you for helping me with this! Do you want me to merge these changes into my own CL or do you want to check this in yourself?
Jul 14 2021
Jul 8 2021
Jan or Tamas, can either of you take a look?
Jul 6 2021
Ping on this
May 17 2021
Can I get a review on this please?
This change is turning out to be less important as we seem to usually avoid building a full manual index if everything it built with -gpubnames. However the other change (https://reviews.llvm.org/D100299) is still necessary to prevent eagerly loading all non-skeleton information. Let's focus on that change and I will revisit this one if it turns out customers are still hitting the manual fallback.
Apr 30 2021
My plan for testing this is not panning out, as lldb provides no error when it fails to load symbols and no way to find out what symbols have been loaded or have been attempted, so there's no way to test that it is behaving lazily. Is there any particular thing you want me to test? Many tests are already hitting this code.
Apr 28 2021
Apr 23 2021
Apr 22 2021
I'll get started on the test, but I'll probably want to use the same test file to test both CLs. What I want to do is write a case that references two dwos, only one of which actually exists, so that I can verify that certain queries can be done without attempting to load the other one.
Before I review this in detail, I'd like to understand your use case better. Do I understand correctly that you want to open a binary, and the _only_ thing you want to do with it is look up a bunch of variables?
Changed contract of DWARFIndex::GetGlobalVariables to take a non-const reference to the skeleton unit and changed each implementation to get the skeleton unit. This allowed further changes to cache by offset using existing m_units_to_avoid rather than cache by dwo id.
Apr 20 2021
Let me know if I should request this review from someone else. It is important for scalability, as this change, in combination with my follow-up change (https://reviews.llvm.org/D100771) eliminate the need to load all .dwo files in the most common debugging scenarios.
Rebased to not depend on other cl
Apr 19 2021
I can combine this with https://reviews.llvm.org/D100299 if it would make reviewing easier.
Apr 13 2021
Not sure what's going on with clang-tidy, but those headers still exist and in any case I didn't add them.
Removing header files to make clang-tidy happy
Apr 12 2021
Feb 24 2021
Ran clang format
I've updated the change to make the cacheing correct. I'm seeing the same query time improvement as with the faulty version. Updated profile:
Fixed cacheing behavior to key by include path rather than include directory index.
Feb 23 2021
Just got back from break. Is there a standard benchmarking suite I should
use for lldb? I have been benchmarking using our own tests on the Chrome
DevTools C++ extension, which uses lldb to read dwarf symbols for wasm
Feb 11 2021
Nov 30 2020
Hope everyone had a nice Thanksgiving break! Are any further changes necessary here?
Nov 25 2020
Nov 24 2020
Okay, I've changed it to call getTombstone everywhere.
Nov 23 2020
Nov 20 2020
Hope this looks better!
Nov 19 2020
Aug 20 2020
Context for this: I am working on an lldb-based debugging tool that needs to be able to generate code to evaluate a dwarf expression without evaluating it. I need access to this method to be able to handle location lists.