[PATCH] Don't calculate whole file crc for ELF core file.

Hi,

Working on something else I’ve noticed that ProcessElfCore plugin can take considerable amount of time when once given big core file.

The file in question was 4.1 GiB core file generated by linux kernel, of process that crashed.

I took the small detour to investigate and it turned out to be spending ~72s
calculating crc32 for whole file from:

ProcessElfCore::CanDebug
ModuleList::GetShareModule

That in the result blocks ObjectFileELF::GetModuleSpecification.

That time was measured with Debug+Assert build variant, on puny i5 laptop with traditional HDD, thus in reality it would be bit better, but still meaningful.

If I understand correctly the main purpose of UUID’s for ObjectFileELF is aiding gnu_debuglink which is used to locate debug info. The other one is related to ModuleSpec and ModuleList where it is used for identification.

First in case of modules doesn’t make much sense, it won’t be correct, or have even possibility to work, while second turns out to be useful, anyway.

Simple workaround or just inventing UUID for ModuleSpec before passing it to ModuleList::GetShareModule would probably work too for this particular case. ELF core files have no universal information reassembling gnu.debuglink or .note.gnu.build-id, thus we will be falling back to quite costly crc computation that is not strictly required.

I have not noticed also easy way to make this computation in background, so I decided limit it to PT_NOTE segments, which still have quite bit of unique info, like: combinations pids/tids/uids/register contexts, and sometimes much more.

That brings down time to somewhat more appealing value:

0.397 ms

Please note that while it was bit optimistic case for that solution, since it was single threaded process, and its PT_NOTE segment had only 4680 bytes.

For other core binary that had PT_NOTE segment of size ~48MiB it still takes:
1.206 s

in same setup.
That was pretty artificial case of fork bomb with almost 30k (29357) threads.

While using only note segment makes hitting collision it bit more likely, I think it is pretty remote, and not meaningful in practice.

Also if anyone has ideas about other/better solution to this problem I will gladly look into it, as performance of core file handling is important for my use-cases.

If this solution is fine for trunk, please commit, I don’t have commit access.

Both raw diff and git path of same change are attached.

Cheers,
/Piotr

Don-t-calculate-whole-file-crc-for-ELF-core-file.diff (12.7 KB)

0001-Don-t-calculate-whole-file-crc-for-ELF-core-file.patch (13.7 KB)

Hi,

Here is rebased version of same patch - so it applies cleanly on top of:

https://llvm.org/svn/llvm-project/lldb/trunk@204545

as it also touches ObjectFileELF.cpp and I somehow missed it yesterday…

Don-t-calculate-whole-file-crc-for-ELF-core-file_v2.diff (12.7 KB)

0001-Don-t-calculate-whole-file-crc-for-ELF-core-file_v2.patch (13.7 KB)

Hey Piotr,

Thanks for taking a look at that. I’ll have a run through it today or tomorrow. I had noticed some really slow backtrace times on relatively simple Linux x86_64 cores but just hadn’t had the cycles to look at it yet.

I’ll get back on this once I have a chance to look at it. (If anybody else has a chance to look at it before me, feel free).

Looks ok, but I _would_ cache the calculated UUID in ObjectFile::m_uuid within the ObjectFileELF::GetUUID() function so they never have to be done again. If other linux folks are happy with this ELF patch, it looks ok to me.

Greg

gnu_debuglink uses the CRC to confirm that a standalone debug file
(e.g. library.so.debug) matches the binary or library, and needs to be
calculated only for those debug files. In fact, prior to the
gnu_debuglink support I think ELF files all ended up with an all-0s
UUID. I think your change is a reasonable first step.

-Ed

I was thinking about core files, not modules - no proof reading on my side
- sorry, and of course you're 100% correct here.
We already got gnu_debuglink crc from binary, and calculated value from
core file makes no sense for that purpose.

What I was thinking about is that would be nice to have "core matches
executable" functionality, and I plan to work at it at some point, but it
is other issue...

Cheers,
/P

Piotr,

I just checked this in after reviewing and testing. It includes the lazy eval and caching that you added in a separate thread with me.

tfiala@tfiala2:/mnt/ssd/work/svn/lgs/llvm/tools/lldb$ svn commit
Waiting for Emacs…
Sending source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
Sending source/Plugins/ObjectFile/ELF/ObjectFileELF.h
Transmitting file data …
Committed revision 204749.

Thanks for improving that, Piotr!

Sincerely,
Todd Fiala