Skip to content

Adjust style-servo patch - #728

Merged
Mark-Simulacrum merged 1 commit into
rust-lang:masterfrom
Mark-Simulacrum:update-style-servo
Jul 27, 2020
Merged

Adjust style-servo patch#728
Mark-Simulacrum merged 1 commit into
rust-lang:masterfrom
Mark-Simulacrum:update-style-servo

Conversation

@Mark-Simulacrum

Copy link
Copy Markdown
Member

This adjusts the style-servo println patch to be in a non-templated file.
Previously, the properties file change would cause the build script to rerun.

This adjusts the style-servo println patch to be in a non-templated file.
Previously, the properties file change would cause the build script to rerun.
@Mark-Simulacrum

Copy link
Copy Markdown
Member Author

We'll likely want to re-collect stable (i.e., dashboard) benchmarks to account for this adjustment to style-servo, but that can be done later when our queue isn't suffering quite as much :)

@Mark-Simulacrum

Copy link
Copy Markdown
Member Author

I suspect that the old patch was technically still fine -- it caused a single-line change to the output of the template. But editing a 141,865 line file means that our current codegen unit partioning strategy is going to recompile a lot more as a result.

@nnethercote I'm going to let you weigh in before we merge this (or not). IMO, this PR brings the patch more inline with what we expect from the println patches -- small edits to small CGUs. Alternatively we can call this a bug in rustc that a small edit to a large module represents a lot of recompilation (maybe related to the generated nature of the file, i.e., that it's all included via a include! macro?).

cc @oli-obk @wesleywiser @pnkfelix who I believe are currently working on incremental / CGU partioning, but feel free to cc others if I've categorized incorrectly.

@Mark-Simulacrum

Copy link
Copy Markdown
Member Author

For reference, here is the generated file: https://gist.github.com/Mark-Simulacrum/9fe9b6fde89c4598c4ae0c13f45bad12

@nnethercote

Copy link
Copy Markdown
Contributor

I don't have a strong preference here. Changing the println location seems reasonable.

@Mark-Simulacrum

Copy link
Copy Markdown
Member Author

Okay, I'll go ahead and merge this then. It should be a slight increase in the instructions stat for the style-servo incremental tests but not as significant as what we saw the drop as.

I'll also work on recollecting stable builds.

@Mark-Simulacrum
Mark-Simulacrum merged commit f53d94e into rust-lang:master Jul 27, 2020
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.

2 participants