Rustdoc : stop cloning html render context

[Wardenfar]

The issue was about cleaning the shared context in html rendering.
The problem was that the Context was clone for each item rendered.

I change the rendering process to not clone the context so the shared context do not require anymore to be in a std::rc::Rc

Feel free to give me feedback
This PR may impact performance: should we run perf-test ?

Resolves #82381

[rust-highfive]

r? @CraftSpider

(rust-highfive has picked a reviewer for you, use r? to override)

[Kobzol]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 89c1ed6e040ff3e811ed1715404ece966238c3e7 with merge b4cf7bf90d47893ae5c2eb77817f496726eb8678...

[bors]

☀️ Try build successful - checks-actions
Build commit: b4cf7bf90d47893ae5c2eb77817f496726eb8678 (b4cf7bf90d47893ae5c2eb77817f496726eb8678)

[rust-timer]

Queued b4cf7bf90d47893ae5c2eb77817f496726eb8678 with parent f03ce30, future comparison URL.

[rust-timer]

Finished benchmarking commit (b4cf7bf90d47893ae5c2eb77817f496726eb8678): comparison url.

Instruction count

• Primary benchmarks: 😿 relevant regressions found
• Secondary benchmarks: no relevant changes found

	mean1	max	count2
Regressions 😿 (primary)	0.9%	1.9%	3
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	0.9%	1.9%	3

Max RSS (memory usage)

Results

• Primary benchmarks: 🎉 relevant improvement found
• Secondary benchmarks: 🎉 relevant improvements found

	mean1	max	count2
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-1.8%	-1.8%	1
Improvements 🎉 (secondary)	-1.8%	-1.8%	2
All 😿🎉 (primary)	-1.8%	-1.8%	1

Cycles

Results

• Primary benchmarks: 😿 relevant regressions found
• Secondary benchmarks: no relevant changes found

	mean1	max	count2
Regressions 😿 (primary)	5.5%	7.8%	2
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	5.5%	7.8%	2

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

Footnotes

1. the arithmetic mean of the percent change ↩ ↩² ↩³

2. number of relevant changes ↩ ↩² ↩³

[Wardenfar]

Do I have to do something ?

[jyn514]

r? @camelid

[bors]

☔ The latest upstream changes (presumably #96869) made this pull request unmergeable. Please resolve the merge conflicts.

[camelid]

Thanks for doing this! I will do my best to look at soon. :)

[bors]

☔ The latest upstream changes (presumably #101238) made this pull request unmergeable. Please resolve the merge conflicts.

[camelid]

I was able to look at this today, and overall it looks very good! I will give it a full review within a few days.

[camelid]

Sorry for taking so long to review this!

My one concern is that you added several RefCells and Cells to the new ContextItemFrame struct and changed &mut Context to &Context everywhere. I don't see why this is necessary, and we've been trying really hard to move away from interior mutability. Is it possible to directly mutate ContextItemFrame through &mut Context instead?

If that's not possible, please let me know and we'll figure out how to go forward.

[Wardenfar]

@camelid
In a lot of places, when removing RefCell<Shared> the code no longer compiles with the borrow checker.
In the rendering/printing process, only the ContextItemFrame is mutated. To remove completely RefCell / Cell, you need to remove the ContextItemFrame from the Context struct (to allow &Context and &mut ContextItemFrame at the same time)

I tried today but Context is everywhere. This is why i choose to keep RefCell (but in a smaller part of the struct)

The other way is to rewrite places where the borrow checker fail, but this is a lot of time.

Thank you for your time !

[camelid]

Thanks for your reply!

Why do we need &Context and &mut ContextItemFrame simultaneously? Can't we have &mut Context and use that to get the item frames? I haven't looked at this code in a while, so apologies if I'm missing something.

[bors]

☔ The latest upstream changes (presumably #102061) made this pull request unmergeable. Please resolve the merge conflicts.

[JohnCSimon]

@Wardenfar
Ping from triage:
Can you address the comment from the reviewer and fix the merge conflicts?

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

[Wardenfar]

@JohnCSimon
Sorry I don't have time to work on it
Should I close the PR ?

[JohnCSimon]

@JohnCSimon Sorry I don't have time to work on it Should I close the PR ?

I'll close it for you.
Please reopen when you are ready to continue with this.
Note: if you do please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive