Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Use the object crate for metadata reading #83640
Use the object crate for metadata reading #83640
Changes from 4 commits
267d55d
f5d3883
3ae15ca
802fe17
b65a92f
487427f
537e814
6381aaf
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I think this (and use of mmap) may be one plausible reason for the slight performance regression. If my memory of the
ar
format serves me well, obtaining the list of members in anar
has to process the file effectively as if it was a ump list. I suspect that such a read pattern may be a pathological for mmap based I/O: kernel would try loading more data (page?) into memory only for us to inspect the file name and length before we jump to the next entry(-ies), discarding the rest of the data that kernel spent time loading in.Without digging into LLVM's ArchiveRO implementation I can imagine that more precise reads could be more effective here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ArchiveRO::open
also uses mmap for header reading I think:rust/compiler/rustc_llvm/llvm-wrapper/ArchiveWrapper.cpp
Line 66 in 6e17a5c
MemoryBufferRef
doesn't export any method allowingread
calls on the mapped file.