Skip to content

/rust

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

incr.comp.: Speed up span hashing by caching expansion context hashes. #46562

Merged
merged 2 commits into from Dec 14, 2017

Conversation

Reviewers

@eddyb eddyb

Assignees

@eddyb eddyb

Labels
S-waiting-on-review
Projects
None yet
Milestone
No milestone
6 participants
@michaelwoerister @rust-highfive @bors @Mark-Simulacrum @eddyb @kennytm
@michaelwoerister
Copy link
Contributor

michaelwoerister commented Dec 7, 2017 •
edited

This PR fixes the performance regressions from #46338.

r? @nikomatsakis

@rust-highfive rust-highfive assigned eddyb Dec 7, 2017

@rust-highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

rust-highfive commented Dec 7, 2017

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@michaelwoerister michaelwoerister unassigned eddyb Dec 7, 2017

@michaelwoerister

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

michaelwoerister commented Dec 7, 2017

@bors try
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Dec 7, 2017

⌛️ Trying commit 1f8678a with merge b867f18...

bors added a commit that referenced this pull request Dec 7, 2017

@bors
Auto merge of #46562 - michaelwoerister:faster-span-hashing, r=<try> 
incr.comp.: Speed up span hashing by caching expansion context hashes.

Please don't merge yet. I want to see the performance impact first.
Loading status checks…
b867f18

@kennytm kennytm added the S-waiting-on-review label Dec 7, 2017

@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Dec 7, 2017

☀️ Test successful - status-travis
State: approved= try=True
@michaelwoerister

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

michaelwoerister commented Dec 7, 2017

@Mark-Simulacrum, could you do a perf run please?

@michaelwoerister michaelwoerister force-pushed the michaelwoerister:faster-span-hashing branch from 1f8678a to fbcf14e Dec 8, 2017

@Mark-Simulacrum

This comment has been minimized.

Sign in to view
Copy link
Member

Mark-Simulacrum commented Dec 9, 2017

Perf run started. Sorry for the delay.
@michaelwoerister

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

michaelwoerister commented Dec 12, 2017

Hey @Mark-Simulacrum, did that perf run ever finish?
@Mark-Simulacrum

This comment has been minimized.

Sign in to view
Copy link
Member

Mark-Simulacrum commented Dec 12, 2017 •
edited

Yes. I generally forget to check back in; they auto-queue and finish within 4 hours at most, and the URLs are fairly easy to generate. Eventually this will be automated...

URL: http://perf.rust-lang.org/compare.html?start=ee25791df5f025378e9ddebef661e25cb75d9172&end=b867f1811e7bb767d248b0ae46b8fabc16e83bed&stat=instructions%3Au
@michaelwoerister

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

michaelwoerister commented Dec 12, 2017

Thanks @Mark-Simulacrum! I see now how I can compose these URLs myself.
@michaelwoerister

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

michaelwoerister commented Dec 12, 2017

This is now ready for review, @nikomatsakis.
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Dec 13, 2017

☔️ The latest upstream changes (presumably #46550) made this pull request unmergeable. Please resolve the merge conflicts.

@michaelwoerister michaelwoerister force-pushed the michaelwoerister:faster-span-hashing branch from fbcf14e to 0b4c2cc Dec 14, 2017

@michaelwoerister

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

michaelwoerister commented Dec 14, 2017

Rebased.
r? @eddyb (@nikomatsakis is a bit swamped right now)

@rust-highfive rust-highfive assigned eddyb Dec 14, 2017

@eddyb

eddyb approved these changes Dec 14, 2017
View changes

@eddyb

This comment has been minimized.

Sign in to view
Copy link
Member

eddyb commented Dec 14, 2017

@bors r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Dec 14, 2017

📌 Commit 0b4c2cc has been approved by eddyb
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Dec 14, 2017

⌛️ Testing commit 0b4c2cc with merge 3fc7f85...

bors added a commit that referenced this pull request Dec 14, 2017

@bors
Auto merge of #46562 - michaelwoerister:faster-span-hashing, r=eddyb 
incr.comp.: Speed up span hashing by caching expansion context hashes.

This PR fixes the performance regressions from #46338.

r? @nikomatsakis
Loading status checks…
3fc7f85
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Dec 14, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing 3fc7f85 to master...

@bors bors merged commit 0b4c2cc into rust-lang:master Dec 14, 2017
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@michaelwoerister michaelwoerister referenced this pull request Dec 15, 2017

Merged

incr.comp.: Revert hashing optimization that caused regression. #46757

bors added a commit that referenced this pull request Dec 16, 2017

@bors
Auto merge of #46757 - michaelwoerister:revert-46562, r=eddyb 
incr.comp.: Revert hashing optimization that caused regression.

This PR reverts part of #46562 which caused [a regression in the crossbeam rust-icci](https://travis-ci.org/rust-icci/crossbeam/builds/316574774) test. I don't know what the problem is exactly yet. Fortunately, the problematic part is also the less important one, so reverting should not have much impact on performance.

r? @eddyb
Loading status checks…
39cb4c6

@alexcrichton alexcrichton referenced this pull request Jan 8, 2018

Closed

Significantly slower --emit=metadata #47267

@michaelwoerister michaelwoerister referenced this pull request Jan 22, 2018

Open

Tracking Issue for Incremental Compilation 2018 #47660

20 of 32 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.