Skip to content
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

Optimize write_metadata. #37267

Merged
merged 1 commit into from
Oct 19, 2016
Merged

Conversation

nnethercote
Copy link
Contributor

@nnethercote nnethercote commented Oct 19, 2016

write_metadata currently generates metadata unnecessarily in some
cases, and also compresses it unnecessarily in some cases. This commit
fixes that. It speeds up three of the rustc-benchmarks by 1--4%.

r? @eddyb, who deserves much of the credit because he (a) identified the problem from the profile data I provided in #37086, and (b) explained to me how to fix it. Thank you, @eddyb!

@nnethercote
Copy link
Contributor Author

Measurements for a stage1 compiler doing debug builds:

futures-rs-test  4.147s vs  4.175s --> 0.993x faster (variance: 1.012x, 1.009x)
helloworld       0.223s vs  0.224s --> 0.992x faster (variance: 1.011x, 1.006x)
html5ever-2016-  5.182s vs  5.129s --> 1.010x faster (variance: 1.006x, 1.008x)
hyper.0.5.0      4.772s vs  4.754s --> 1.004x faster (variance: 1.012x, 1.007x)
inflate-0.1.0    4.524s vs  4.512s --> 1.003x faster (variance: 1.003x, 1.020x)
issue-32062-equ  0.323s vs  0.322s --> 1.003x faster (variance: 1.009x, 1.023x)
issue-32278-big  1.653s vs  1.661s --> 0.995x faster (variance: 1.004x, 1.009x)
jld-day15-parse  1.460s vs  1.462s --> 0.999x faster (variance: 1.016x, 1.009x)
piston-image-0. 11.318s vs 11.308s --> 1.001x faster (variance: 1.007x, 1.063x)
regex.0.1.30     2.414s vs  2.312s --> 1.044x faster (variance: 1.013x, 1.019x)
rust-encoding-0  1.990s vs  1.979s --> 1.005x faster (variance: 1.008x, 1.007x)
syntex-0.42.2   29.514s vs 29.446s --> 1.002x faster (variance: 1.013x, 1.008x)
syntex-0.42.2-i 15.318s vs 14.959s --> 1.024x faster (variance: 1.059x, 1.006x)

So it's a big win for regex, a moderate win for syntax-incr, a small win for html5ever, and makes little difference to the others. (I did multiple timing runs and these outcomes were consistent.)

.iter()
.map(to_kind)
.max()
.unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

This could be written on one line, with the closure passed to it directly, and with no explicit type on the closure argument.
.max().unwrap() would come on the same line after the }) where the } is the closing bracket of the closure.
Also, you might want to do .unwrap_or(MetadataKind::None) instead of .unwrap() (cc @alexcrichton).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed the indentation.

@alexcrichton: the specific question is this: in write_metadata, can cx.sess().crate_types be empty?

`write_metadata` currently generates metadata unnecessarily in some
cases, and also compresses it unnecessarily in some cases. This commit
fixes that. It speeds up three of the rustc-benchmarks by 1--4%.
config::CrateTypeDylib |
config::CrateTypeProcMacro => MetadataKind::Compressed,
}
}).max().unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

@alexcrichton r=me if the unwrap isn't wrong here.

@alexcrichton
Copy link
Member

@bors: r=eddyb

On Tuesday, October 18, 2016, Eduard-Mihai Burtescu <
notifications@github.com> wrote:

@eddyb commented on this pull request.

In src/librustc_trans/base.rs
#37267 (review):

  •    Uncompressed,
    
  •    Compressed
    
  • }
  • let kind = cx.sess().crate_types.borrow().iter().map(|ty| {
  •    match *ty {
    
  •        config::CrateTypeExecutable |
    
  •        config::CrateTypeStaticlib |
    
  •        config::CrateTypeCdylib => MetadataKind::None,
    
  •        config::CrateTypeRlib => MetadataKind::Uncompressed,
    
  •        config::CrateTypeDylib |
    
  •        config::CrateTypeProcMacro => MetadataKind::Compressed,
    
  •    }
    
  • }).max().unwrap();

@alexcrichton https://github.com/alexcrichton r=me if the unwrap isn't
wrong here.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#37267 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAD95KDALZ8ApEOShtYia3TaL_UNoQbaks5q1ZbjgaJpZM4KagGJ
.

@bors
Copy link
Contributor

bors commented Oct 19, 2016

📌 Commit 016f69f has been approved by eddyb

eddyb added a commit to eddyb/rust that referenced this pull request Oct 19, 2016
…ddyb

Optimize `write_metadata`.

`write_metadata` currently generates metadata unnecessarily in some
cases, and also compresses it unnecessarily in some cases. This commit
fixes that. It speeds up three of the rustc-benchmarks by 1--4%.

r? @eddyb, who deserves much of the credit because he (a) identified the problem from the profile data I provided in rust-lang#37086, and (b) explained to me how to fix it. Thank you, @eddyb!
bors added a commit that referenced this pull request Oct 19, 2016
@bors bors merged commit 016f69f into rust-lang:master Oct 19, 2016
@nnethercote nnethercote deleted the opt-write_metadata branch October 19, 2016 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants