Page MenuHomePhabricator

[SymbolFileDWARF] Replace FixedFormSizes with llvm::dwarf::getFixedFormByteSize
AbandonedPublic

Authored by labath on Mar 21 2018, 8:13 AM.

Details

Summary

The llvm function is practically a drop-in replacement. There may be
slight differences in performance as the llvm version uses a switch
whereas our version used a lookup table, but I can't say which way that
will go. (In practice, the compiler will also turn the switch into a jump
table.)

The thing we definitely gain by this is support for new forms that
haven't been added to our tables (e.g. DW_FORM_data16) and the ability
to represent forms of size 0 like DW_FORM_implicit_const (which we
couldn't represent because we used 0 to mean "size unknown"). The thing
we definitely lose is 150 lines of code.

Event Timeline

labath created this revision.Mar 21 2018, 8:13 AM

Looks fine. It would be nice to verify that there are no performance regressions due to this before making this change. Can you do some timings to make sure? Do anything that indexes a large C++ DWARF codebase, like clang, and make sure we don't regress by a significant amount.

source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
765–766

formatting?

Hmm... how much is a "significant"? I tried time optimized/lldb --file debug/clang -o "br set -n dump" as a benchmark and I saw a 2-3% slowdown (the variance between individual samples is ~1%, so this is statistically significant).

I tried playing around with this, and I was able to get about 1% speedup (borderline significant) by changing the llvm's implementation to a lookup table (presumably, the speedup comes from the fact that the llvm version handles more forms, most likely the ones whose size is dwarf-version-dependent). However, this required making some non-llvm-y changes like ditching llvm::Optional as a return type, as that saved a couple of instructions (I should probably double check that with a top-of-tree compiler, to see if it can optimize it better).

I would like to not lose any performance if possible.

labath abandoned this revision.Mar 26 2018, 5:57 AM

I'm not sure my benchmark strategy is correct here. I'm getting different (but consistent) results even after making changes that should not impact performance at all (e.g. because I change the code which is not executed). I'll abandon this until I find some time to benchmark this properly.