-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Streamline Compiler
#64016
Streamline Compiler
#64016
Changes from all commits
d61605c
cd0c21b
d264a56
2521189
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,29 +114,38 @@ impl Compiler { | |
let crate_name = self.crate_name()?.peek().clone(); | ||
let krate = self.parse()?.take(); | ||
|
||
passes::register_plugins( | ||
self, | ||
let result = passes::register_plugins( | ||
self.session(), | ||
self.cstore(), | ||
krate, | ||
&crate_name, | ||
) | ||
); | ||
|
||
// Compute the dependency graph (in the background). We want to do | ||
// this as early as possible, to give the DepGraph maximum time to | ||
// load before dep_graph() is called, but it also can't happen | ||
// until after rustc_incremental::prepare_session_directory() is | ||
// called, which happens within passes::register_plugins(). | ||
self.dep_graph_future().ok(); | ||
|
||
result | ||
}) | ||
} | ||
|
||
pub fn crate_name(&self) -> Result<&Query<String>> { | ||
self.queries.crate_name.compute(|| { | ||
let parse_result = self.parse()?; | ||
let krate = parse_result.peek(); | ||
let result = match self.crate_name { | ||
Ok(match self.crate_name { | ||
Some(ref crate_name) => crate_name.clone(), | ||
None => rustc_codegen_utils::link::find_crate_name( | ||
Some(self.session()), | ||
&krate.attrs, | ||
&self.input | ||
), | ||
}; | ||
Ok(result) | ||
None => { | ||
let parse_result = self.parse()?; | ||
let krate = parse_result.peek(); | ||
rustc_codegen_utils::link::find_crate_name( | ||
Some(self.session()), | ||
&krate.attrs, | ||
&self.input | ||
) | ||
} | ||
}) | ||
}) | ||
} | ||
|
||
|
@@ -194,7 +203,6 @@ impl Compiler { | |
|
||
pub fn prepare_outputs(&self) -> Result<&Query<OutputFilenames>> { | ||
self.queries.prepare_outputs.compute(|| { | ||
self.lower_to_hir()?; | ||
let krate = self.expansion()?; | ||
let krate = krate.peek(); | ||
let crate_name = self.crate_name()?; | ||
|
@@ -267,6 +275,11 @@ impl Compiler { | |
}) | ||
} | ||
|
||
// This method is different to all the other methods in `Compiler` because | ||
// it lacks a `Queries` entry. It's also not currently used. It does serve | ||
// as an example of how `Compiler` can be used, with additional steps added | ||
// between some passes. And see `rustc_driver::run_compiler` for a more | ||
// complex example. | ||
pub fn compile(&self) -> Result<()> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is meant as an simple utility function and an example for users of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If an example is needed, why not provide it in documentation? Don't leave unnecessary code in the product. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have CI testable example code snippets in the rustc-guide?
If either of the above is true, then I could go along with @nnethercote 's reasoning here, though then I would want that migration to happen either before this PR, or as part of this PR. (Otherwise, I'm with @Zoxc; Its hard enough for people to get started using the compilier as a library or hacking on |
||
self.prepare_outputs()?; | ||
|
||
|
@@ -278,12 +291,12 @@ impl Compiler { | |
|
||
self.global_ctxt()?; | ||
|
||
// Drop AST after creating GlobalCtxt to free memory | ||
// Drop AST after creating GlobalCtxt to free memory. | ||
mem::drop(self.expansion()?.take()); | ||
|
||
self.ongoing_codegen()?; | ||
|
||
// Drop GlobalCtxt after starting codegen to free memory | ||
// Drop GlobalCtxt after starting codegen to free memory. | ||
mem::drop(self.global_ctxt()?.take()); | ||
|
||
self.link().map(|_| ()) | ||
|
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.
Moving the dep graph loading to a later point is a performance regression. This is currently done as early as possible, although I'd like for it be done immediately when creating the compiler context, but that requires some changes as currently we need to do this after parsing to pick up the crate name.
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.
The performance run shows that these five commits provide a slightly performance improvement.