Move NativeLib::filename to the rmeta-link archive member#156735
Open
mehdiakiki wants to merge 1 commit into
Open
Move NativeLib::filename to the rmeta-link archive member#156735mehdiakiki wants to merge 1 commit into
mehdiakiki wants to merge 1 commit into
Conversation
Collaborator
|
rustbot has assigned @dingxiangfei2009. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
053432f to
8f96681
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
8f96681 to
5a93c75
Compare
Contributor
Author
|
r? @petrochenkov |
5a93c75 to
3b0d8b8
Compare
Contributor
Author
|
Started working on it today. Should have it ready soon. |
3b0d8b8 to
e923f64
Compare
This comment has been minimized.
This comment has been minimized.
e923f64 to
2414bd9
Compare
This comment has been minimized.
This comment has been minimized.
2414bd9 to
b0da354
Compare
This comment has been minimized.
This comment has been minimized.
b0da354 to
92610ca
Compare
This comment has been minimized.
This comment has been minimized.
92610ca to
70f18ca
Compare
This comment has been minimized.
This comment has been minimized.
70f18ca to
578ba56
Compare
This comment has been minimized.
This comment has been minimized.
b8f2284 to
b0497c9
Compare
Collaborator
|
This PR modifies |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
b0497c9 to
c17c40e
Compare
Collaborator
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
petrochenkov
reviewed
Jul 3, 2026
| } | ||
|
|
||
| // FIXME: this is mostly a copy-paste of `DefaultMetadataLoader::get_rlib_metadata`. | ||
| pub fn read_from_path(target: &Target, path: &Path) -> Option<RmetaLink> { |
Contributor
There was a problem hiding this comment.
When we call this function we always do this
.map(|rl| {
rl.native_lib_filenames
.iter()
.map(|f| f.as_deref().map(Symbol::intern))
.collect()
})
.unwrap_or_default()as well.
Could you merge that logic into this function, or add a second function doing read_from_path + the logic above.
| let relevant = native_libs.clone().filter(|lib| relevant_lib(sess, lib)); | ||
| let relevant_libs: FxIndexSet<_> = relevant.filter_map(|lib| lib.filename).collect(); | ||
| let native_libs = &crate_info.native_libraries[&cnum]; | ||
| let filenames: Vec<Option<Symbol>> = if crate_may_have_bundled_libs(native_libs) { |
Contributor
There was a problem hiding this comment.
Suggested change
| let filenames: Vec<Option<Symbol>> = if crate_may_have_bundled_libs(native_libs) { | |
| let bundled_filenames: Vec<Option<Symbol>> = if crate_may_have_bundled_libs(native_libs) { |
| let native_libs = match cnum { | ||
| LOCAL_CRATE => &crate_info.used_libraries, | ||
| _ => &crate_info.native_libraries[&cnum], | ||
| let (native_libs, filenames): (&Vec<NativeLib>, Vec<Option<Symbol>>) = match cnum { |
Contributor
There was a problem hiding this comment.
Suggested change
| let (native_libs, filenames): (&Vec<NativeLib>, Vec<Option<Symbol>>) = match cnum { | |
| let (native_libs, bundled_filenames): (&Vec<NativeLib>, Vec<Option<Symbol>>) = match cnum { |
Contributor
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rust-bors Bot
pushed a commit
that referenced
this pull request
Jul 3, 2026
Move NativeLib::filename to the rmeta-link archive member
c17c40e to
6ce6345
Compare
Contributor
This comment has been minimized.
This comment has been minimized.
Collaborator
|
Finished benchmarking commit (2f75410): comparison URL. Overall result: no relevant changes - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)This perf run didn't have relevant results for this metric. CyclesResults (secondary -2.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 485.944s -> 486.779s (0.17%) |
Contributor
Author
|
Resolved the conflicts |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
View all comments
Second PR in #138243
Moves
NativeLib::filenameout ofrmetaintolib.rmeta-linkarchive member that was introduced in the first PR. Filename is a link time only data so requiring a full metadata decode should be avoided. It is stored as(name, filename)pairs keyed by name, the newMetadataLoader::get_rlib_native_lib_filenamespatches it back on decode. Also bumpedMETADATA_VERSIONfrom version 10 to 11. Added also new round trip test and existing bundled-libs tests still pass.