rustc_span: add span_data_to_lines_and_cols to caching source map view #79012
Conversation
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
This is a performance mitigation for #76256. The improvement is about the smallest I'd feel justified in creating PR for, and I'm not totally sure the improvement justifies the added complexity. But I'm doing my due diligence. |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 86a35a4b64dff6b358e7513012985387c24507c8 with merge dbf6d2754823752951e168718a5fa4b318adf742... |
|
☀️ Try build successful - checks-actions |
|
Queued dbf6d2754823752951e168718a5fa4b318adf742 with parent f036a8f, future comparison URL. |
|
Finished benchmarking try commit (dbf6d2754823752951e168718a5fa4b318adf742): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
Note: this seems to have very small perf improvements. |
|
Yeah, I don't have strong feelings on whether or not to push forward with this. I'll note that cycles, cpu-clock, and task-clock stats saw moderate improvements even though instruction counts saw very small improvements. But it's possible that was just luck with the variance. I'll rebase and get another perf run. If those stats show improvements again, I'd feel stronger about trying to merge this. |
Gives a performance increase over calling byte_pos_to_line_and_col twice, partially because it decreases the function calling overhead, potentially because it doesn't populate the line cache with lines that turn out to belong to invalid spans, and likely because of some other incidental improvements made possible by having more context available.
tgnottingham
force-pushed
the
span_data_to_lines_and_cols
branch
from
86a35a4
to
75de828
Compare
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 75de828 with merge 57b7048fd4b5f30335c7a76b1c80b42e1c630b52... |
|
☀️ Try build successful - checks-actions |
|
Queued 57b7048fd4b5f30335c7a76b1c80b42e1c630b52 with parent e9dd18c, future comparison URL. |
|
Finished benchmarking try commit (57b7048fd4b5f30335c7a76b1c80b42e1c630b52): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
The second perf run was a lot more promising :) up to -.7% on instruction counts. |
|
@bors r+ rollup=never |
|
📌 Commit 75de828 has been approved by |
bors
added
S-waiting-on-bors
|
CC @rust-lang/compiler |
|
☀️ Test successful - checks-actions |
No description provided.