Skip to content

[Linux] Improve backtracer ELF/DWARF parsing performance. #69603

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

Merged
merged 3 commits into from
Nov 6, 2023

Conversation

al45tair
Copy link
Contributor

@al45tair al45tair commented Nov 2, 2023

On Linux we use FileImageSource to read data from ELF images. We were doing this in a fairly naive manner, which turns out to be a performance issue with larger statically linked binaries on Linux.

Change FileImageSource to use mmap() instead.

rdar://117590155

On Linux we use FileImageSource to read data from ELF images.  We were
doing this in a fairly naive manner, which turns out to be a performance
issue with larger statically linked binaries on Linux.

Change FileImageSource to use mmap() instead.

rdar://117590155
We don't really need it after calling `mmap()`, and not keeping it around
means it's easy to make sure it gets closed properly in all cases.

rdar://117590155
@al45tair al45tair added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.10 labels Nov 2, 2023
@al45tair al45tair requested a review from a team as a code owner November 2, 2023 11:47
@al45tair
Copy link
Contributor Author

al45tair commented Nov 2, 2023

@swift-ci Please test

@al45tair
Copy link
Contributor Author

al45tair commented Nov 2, 2023

Explanation: On Linux, we read data from ELF files using FileImageSource, which implements the ImageSource and MemoryReader protocols to allow code to fetch data from the image easily. The existing implementation is naïve and relies solely on the buffer cache in the OS to provide reasonable performance. Changing the implementation to use mmap() instead yields an approximately 4x speed-up. Some of the server-side Swift developers churn out statically linked single binaries in the hundreds of MB range where simply parsing the DWARF data from the image takes significant numbers of seconds, hence the interest in this for a 5.9 update.
Original PR: #69508
Reviewed by: @mikeash
Resolves: rdar://117590155
Tests: The existing backtracing tests exercise this code.

It was pointed out that there was a more Swift-y way to write the
bounds checks for the `fetch<T>` implementation, so do that instead.

rdar://117590155
@al45tair
Copy link
Contributor Author

al45tair commented Nov 3, 2023

@swift-ci Please test

@al45tair al45tair merged commit bf3aa49 into swiftlang:release/5.10 Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants