Skip to content
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.

Sign up for GitHub

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

Jump to bottom

impl core::fmt::Write for rustdoc::html::render::Buffer #92563

Closed
jyn514 opened this issue Jan 4, 2022 · 2 comments · Fixed by #92758
Closed

impl core::fmt::Write for rustdoc::html::render::Buffer #92563

jyn514 opened this issue Jan 4, 2022 · 2 comments · Fixed by #92758
Assignees
@mfrw
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. I-compiletime Issue: Problems and improvements with respect to compile times. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Jan 4, 2022 •
edited by rustbot
Loading

A further performance improvement here would be to make sure that Buffer impls core::fmt::Write, in which case we could use render_into(buf) rather than render(), avoiding the allocation of a temporary String here. Buffer internally seems to just hold a String and it has write_str() and write_fmt() methods already, so perhaps it would make sense to implement that trait? I'm not sure how big/relevant the performance improvement would be.

Originally posted by @djc in #92526 (comment)
@jyn514 jyn514 added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 4, 2022
@djc
Copy link
Contributor

djc commented Jan 4, 2022

Since askama::Template implementers get a std::fmt::Display generated for them by Askama's macro, the simpler way to get the meat of this change would probably be to call buf.write_fmt(format_args!("{}", tmpl));

@camelid camelid added I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. I-compiletime Issue: Problems and improvements with respect to compile times. labels Jan 5, 2022
@mfrw
Copy link
Contributor

mfrw commented Jan 11, 2022

@rustbot claim
Let me take a stab at it :)

@mfrw mfrw mentioned this issue Jan 11, 2022
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 2, 2022
@matthiaskrgr
Rollup merge of rust-lang#92758 - mfrw:mfrw/rustdoc-impl-write, r=Gui…
db6950f 
…llaumeGomez

librustdoc: impl core::fmt::Write for rustdoc::html::render::Buffer

Signed-off-by: Muhammad Falak R Wani <[email protected]>

Fixes: rust-lang#92563
@bors bors closed this as completed in 7117b45 Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mfrw mfrw

Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. I-compiletime Issue: Problems and improvements with respect to compile times. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

librustdoc: impl core::fmt::Write for rustdoc::html::render::Buffer mfrw/rust
4 participants
@djc @mfrw @jyn514 @camelid