Don't copy bytecode files into the incr. comp. cache.#71754
Don't copy bytecode files into the incr. comp. cache.#71754bors merged 1 commit intorust-lang:masterfrom
Conversation
It's no longer necessary now that bitcode is embedded into object files. This change meant that `WorkProductFileKind::Bytecode` is no longer necessary, which means that type is no longer necessary, which allowed several places in the code to become simpler.
|
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @nnethercote |
|
Looks fine, but I'd like to see a perf run first, just to know if it has much effect. @bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit d4e5e1b with merge 032fb238cef1c57960742f33db0829b5f52a23a1... |
|
☀️ Try build successful - checks-azure |
|
Queued 032fb238cef1c57960742f33db0829b5f52a23a1 with parent 7f65393, future comparison URL. |
|
Finished benchmarking try commit 032fb238cef1c57960742f33db0829b5f52a23a1, comparison URL. |
|
Performance looks essentially neutral, at least by instruction counts - which is what we would expect, I think, as copying into the incr comp cache isn't heavy on instructions presumably. @bors r=nnethercote |
|
📌 Commit d4e5e1b has been approved by |
bors
added
S-waiting-on-bors
|
The wall time results are noisy as always, but look like a slight improvement. |
|
☀️ Test successful - checks-azure |
| pub enum WorkProductFileKind { | ||
| Object, | ||
| Bytecode, | ||
| pub saved_files: Vec<String>, |
There was a problem hiding this comment.
This could become Option<String> as only a single object file will ever be saved for a codegen unit.
8 participants
It's no longer necessary now that bitcode is embedded into object files.
This change meant that
WorkProductFileKind::Bytecodeis no longernecessary, which means that type is no longer necessary, which allowed
several places in the code to become simpler.
This commit was written by @nnethercote in #70458 but that didn't land. In the meantime though we managed to land it in #71528 and that doesn't seem to be causing too many fires, so I'm re-sending this patch!