[Perf] Optimize documentation lints a lot (1/2) (18% -> 10%) #14693

blyxyas · 2025-04-25T18:36:14Z

Turns out that doc_markdown uses a non-cheap rustdoc function to convert from markdown ranges into source spans. And it was using it a lot (about once every 17 lines of documentation on tokio, which ends up being about 2000 times).

This ended up being about 18% of the total Clippy runtime as discovered by lintcheck --perf in docs-heavy crates. This PR optimizes one of the cases in which Clippy calls the function, and a future PR once pulldown-cmark/pulldown-cmark#1034 is merged will be opened. This PR lands the use of the function into the single-digit zone.

Note that not all crates were affected by this crate equally, those with more docs are affected far more than those light ones.

changelog:[clippy::doc_markdown] has been optimized by 50%

rustbot · 2025-04-25T18:36:19Z

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Turns out that `doc_markdown` uses a non-cheap rustdoc function to convert from markdown ranges into source spans. And it was using it a lot (about once every 18 lines of documentation on `tokio`, which ends up being about 1800 times). This ended up being about 18% of the total Clippy runtime as discovered by lintcheck --perf in docs-heavy crates. This PR optimizes one of the cases in which Clippy calls the function, and a future PR once pulldown-cmark/pulldown-cmark issue number 1034 is merged will be open. Note that not all crates were affected by this crate equally, those with more docs are affected far more than those light ones.

Jarcho · 2025-04-26T00:41:40Z

clippy_lints/src/doc/markdown.rs

+        let Some(fragment_span) = fragments.span(cx, range.clone()) else {
+            return ControlFlow::Break(());
+        };
+
+        let span = Span::new(
+            fragment_span.lo() + BytePos::from_usize(fragment_offset),
+            fragment_span.lo() + BytePos::from_usize(fragment_offset + word.len()),
+            fragment_span.ctxt(),
+            fragment_span.parent(),
+        );
+


Should you not be adjusting the range before creating the span? fragment_offset looks like it's an offset in the markdown text.

I'm not sure if I understand this comment correctly. This snippet is taken as-is from check with variable names fixed, check->offset didn't really care about the markdown text.

fragment_offset looks like it's an offset in the cooked doc string. It can't be used as an offset for a span since that doesn't always line up perfectly with the source text.

After testing this out, text_to_check only contains text, it doesn't contain links, or bold text, etc. And fragment_offset is resetted for each one of those texts. I can add a debug assertion for future proofing this though.

A text fragment can still contain escape sequences e.g. #[doc = "docs with unicode \u{xxxxxx}"]. The string the fragments work on is the cooked version of the doc string, not the source form. Multiline comments (/** */) might also have issues, don't know how the those are presented.

Just tested this, #[doc = "*"] does not lint, at all. Even for the test cases documented, if you change the /// * for #[doc = "*"] they will just not lint.

For the case of /** XXX */, seems that everything works correctly (or that I'm testing for the wrong thing), either way, I've added some tests for this along with some other weird escaped sequences.

Don't really want to stall the PR on this and it's not a new thing added. I'll try to make it break later.

Jarcho · 2025-04-26T00:42:40Z

clippy_lints/src/doc/markdown.rs

@@ -117,6 +134,17 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, b
        // try to get around the fact that `foo::bar` parses as a valid URL
        && !url.cannot_be_a_base()
    {
+        let Some(fragment_span) = fragments.span(cx, range.clone()) else {
+            return ControlFlow::Break(());


This seems wrong. One spot failing to get a span doesn't mean all the others will.

Jarcho · 2025-04-26T00:43:04Z

clippy_lints/src/doc/markdown.rs

+        let Some(fragment_span) = fragments.span(cx, range.clone()) else {
+            return ControlFlow::Break(());
+        };
+
+        let span = Span::new(
+            fragment_span.lo() + BytePos::from_usize(fragment_offset),
+            fragment_span.lo() + BytePos::from_usize(fragment_offset + word.len()),
+            fragment_span.ctxt(),
+            fragment_span.parent(),
+        );


Same as the previous two comments.

clippy_lints/src/doc/markdown.rs

Jarcho

Thank you.

@rustbot

So, after rust-lang#14693 was merged, this is the continuation. It performs some optimizations on `Fragments::span` , makes it so we don't call it so much, and makes a 85.75% decrease (7.51% -> 10.07%) in execution samples of `source_span_for_markdown_range` and a 6.39% -> 0.88% for `core::StrSearcher::new`. Overall a 13.11% icount decrase on docs-heavy crates. Benchmarked mainly on `regex-1.10.5`. @rustbot label +performance-project This means that currently our heaviest function is `rustc_middle::Interners::intern_ty`, even for documentation-heavy crates

@rustbot

So, after rust-lang#14693 was merged, this is the continuation. It performs some optimizations on `Fragments::span` , makes it so we don't call it so much, and makes a 85.75% decrease (7.51% -> 10.07%) in execution samples of `source_span_for_markdown_range` and a 6.39% -> 0.88% for `core::StrSearcher::new`. Overall a 13.11% icount decrase on docs-heavy crates. Benchmarked mainly on `regex-1.10.5`. @rustbot label +performance-project This means that currently our heaviest function is `rustc_middle::Interners::intern_ty`, even for documentation-heavy crates

So, after rust-lang#14693 was merged, this is the continuation. It performs some optimizations on `Fragments::span` , makes it so we don't call it so much, and makes a 85.75% decrease (7.51% -> 10.07%) in execution samples of `source_span_for_markdown_range` and a 6.39% -> 0.88% for `core::StrSearcher::new`. Overall a 13.11% icount decrase on docs-heavy crates. Benchmarked mainly on `regex-1.10.5`. This means that currently our heaviest function is `rustc_middle::Interners::intern_ty`, even for documentation-heavy crates

So, after rust-lang#14693 was merged, this is the continuation. It performs some optimizations on `Fragments::span` , makes it so we don't call it so much, and makes a 85.75% decrease (7.51% -> 10.07%) in execution samples of `source_span_for_markdown_range` and a 6.39% -> 0.88% for `core::StrSearcher::new`. Overall a 13.11% icount decrase on docs-heavy crates. Benchmarked mainly on `regex-1.10.5`. This means that currently our heaviest function is `rustc_middle::Interners::intern_ty`, even for documentation-heavy crates Co-authored-by: Roope Salmi <[email protected]>

@rustbot

So, after #14693 was merged, this is the continuation. It performs some optimizations on `Fragments::span` , makes it so we don't call it so much, and makes a 85.75% decrease (7.51% -> 1.07%) in execution samples of `source_span_for_markdown_range` and a 6.39% -> 0.88% for `core::StrSearcher::new`. Overall a 13.11% icount decrase on docs-heavy crates. Benchmarked mainly on `regex-1.10.5`. @rustbot label +performance-project This means that currently our heaviest function is `rustc_middle::Interners::intern_ty`, even for documentation-heavy crates Along with #14693, this makes the lint a 7% of what it was before and makes it so that even in the most doc-heavy of crates it's not an issue. changelog:Optimize documentation lints by a further 85% r? @Jarcho

blyxyas added the performance-project For issues and PRs related to the Clippy Performance Project label Apr 25, 2025

rustbot assigned Jarcho Apr 25, 2025

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 25, 2025

This comment has been minimized.

Sign in to view

blyxyas force-pushed the optimize-doc-lints branch from fe7ec9b to 565cf5a Compare April 25, 2025 19:15

Jarcho reviewed Apr 26, 2025

View reviewed changes

blyxyas mentioned this pull request Apr 29, 2025

Optimizing Clippy & linting rust-lang/rust-project-goals#114

Open

2 tasks

Review comments & Add testing

acff5d3

blyxyas force-pushed the optimize-doc-lints branch from 4a99a06 to acff5d3 Compare May 21, 2025 21:44

Jarcho approved these changes May 21, 2025

View reviewed changes

Jarcho added this pull request to the merge queue May 21, 2025

Merged via the queue into rust-lang:master with commit a6e40fa May 21, 2025
11 checks passed

blyxyas mentioned this pull request May 22, 2025

Optimize documentation lints **a lot** (2/2) (7.5% -> 1%) #14870

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Perf] Optimize documentation lints a lot (1/2) (18% -> 10%) #14693

[Perf] Optimize documentation lints a lot (1/2) (18% -> 10%) #14693

Uh oh!

blyxyas commented Apr 25, 2025

Uh oh!

rustbot commented Apr 25, 2025

Uh oh!

This comment has been minimized.

Jarcho Apr 26, 2025

Uh oh!

blyxyas May 10, 2025

Uh oh!

Jarcho May 10, 2025 •

edited

Loading

Uh oh!

blyxyas May 10, 2025

Uh oh!

Jarcho May 11, 2025

Uh oh!

blyxyas May 21, 2025

Uh oh!

Jarcho May 21, 2025

Uh oh!

Jarcho Apr 26, 2025

Uh oh!

blyxyas May 21, 2025

Uh oh!

Jarcho Apr 26, 2025

Uh oh!

Uh oh!

Jarcho left a comment

Uh oh!

Uh oh!

Uh oh!

[Perf] Optimize documentation lints **a lot** (1/2) (18% -> 10%) #14693

[Perf] Optimize documentation lints **a lot** (1/2) (18% -> 10%) #14693

Uh oh!

Conversation

blyxyas commented Apr 25, 2025

Uh oh!

rustbot commented Apr 25, 2025

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jarcho May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jarcho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[Perf] Optimize documentation lints a lot (1/2) (18% -> 10%) #14693

[Perf] Optimize documentation lints a lot (1/2) (18% -> 10%) #14693

Jarcho May 10, 2025 •

edited

Loading