Skip to content

Conversation

rylev
Copy link
Member

@rylev rylev commented Jan 26, 2021

This stores pre-canacolized paths inside ExternLocation::ExactPaths so that we don't need to canoncalize them every time we want to compare them to source lib paths.

This is related to #81414.

@rust-highfive
Copy link
Contributor

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 26, 2021
@rust-log-analyzer

This comment has been minimized.

@petrochenkov petrochenkov self-assigned this Jan 26, 2021
@nagisa
Copy link
Member

nagisa commented Jan 26, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@bors
Copy link
Collaborator

bors commented Jan 26, 2021

⌛ Trying commit 49971f3d05440d55b102cb310c5528716f9f5cfe with merge 587b64557d05ed718c8fd6e098d3c9b02dc51e8b...

@bors
Copy link
Collaborator

bors commented Jan 26, 2021

☀️ Try build successful - checks-actions
Build commit: 587b64557d05ed718c8fd6e098d3c9b02dc51e8b (587b64557d05ed718c8fd6e098d3c9b02dc51e8b)

@rust-timer
Copy link
Collaborator

Queued 587b64557d05ed718c8fd6e098d3c9b02dc51e8b with parent d1aed50, future comparison URL.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 26, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (587b64557d05ed718c8fd6e098d3c9b02dc51e8b): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 27, 2021
@rylev
Copy link
Member Author

rylev commented Jan 27, 2021

Looks like the perf change is positive albeit minimal. This does have a large effect on compiling the Windows API surface using windows-rs, where it has improved performance considerably (by ~4.5%).

Copy link
Member

@nagisa nagisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, giving @petrochenkov some time to look at this too, since they self-assigned.

@oli-obk oli-obk removed their assignment Jan 27, 2021
@petrochenkov
Copy link
Contributor

This is breaking the logic in fn find_commandline_library that wants to work with the originally passed file names and not their canonicalized versions, see e.g. #75548 for one such case.

So we'll probably have to keep both the originally passed strings and their canonicalized versions.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 27, 2021
@petrochenkov
Copy link
Contributor

Also, with this PR canonicalization performed by fn find_commandline_library is no longer necessary and can be removed.

@bors

This comment has been minimized.

@rylev rylev force-pushed the canocalize-extern-entries branch from 5e7b548 to 2a039e3 Compare January 28, 2021 11:08
@rylev
Copy link
Member Author

rylev commented Jan 28, 2021

@petrochenkov I've refactored so that both the original and canonicalized forms are kept and can be used. This seems to have additional performance gains - my test repo is now compiling somewhat faster than before.

@rylev
Copy link
Member Author

rylev commented Jan 28, 2021

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 28, 2021
@petrochenkov
Copy link
Contributor

r=me after addressing #81419 (comment) and squashing commits.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 28, 2021
@rylev rylev force-pushed the canocalize-extern-entries branch from 30b9fc9 to 6c7ecd0 Compare January 29, 2021 10:02
@rylev
Copy link
Member Author

rylev commented Jan 29, 2021

Addressed the issue and rebased.

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 29, 2021
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 29, 2021

📌 Commit 6c7ecd0 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 29, 2021
@bors
Copy link
Collaborator

bors commented Jan 29, 2021

⌛ Testing commit 6c7ecd0 with merge c4e33b5...

@bors bors mentioned this pull request Jan 29, 2021
@bors
Copy link
Collaborator

bors commented Jan 29, 2021

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing c4e33b5 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 29, 2021
@bors bors merged commit c4e33b5 into rust-lang:master Jan 29, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 29, 2021
@rylev rylev deleted the canocalize-extern-entries branch January 29, 2021 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.