Simplify code to generate line numbers in highlight #148171

GuillaumeGomez · 2025-10-27T13:35:52Z

Follow-up of #146992.

GuillaumeGomez · 2025-10-27T13:36:23Z

I don't think it'll impact performance but just in case:

Simplify code to generate line numbers in highlight

rust-bors · 2025-10-27T15:48:57Z

☀️ Try build successful (CI)
Build commit: b384be5 (b384be56b050599ec6a78499b3413da442fb9150, parent: 34a8c7368c84fc699fc83a8851a02f93fd655931)

rust-timer · 2025-10-27T17:33:48Z

Finished benchmarking commit (b384be5): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.4%	[-0.5%, -0.2%]	7
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary 4.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.1%	[3.3%, 4.6%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 473.665s -> 475.551s (0.40%)
Artifact size: 390.50 MiB -> 390.48 MiB (-0.00%)

GuillaumeGomez · 2025-10-30T14:18:29Z

It's actually faster. I'm surprised. Oh well, good surprises are welcome I guess. 😆

lolbinarycat · 2025-10-30T17:07:03Z

I'm not surprised at all, it's removing an intermediate allocation, I would expect that to improve performance slightly.

lolbinarycat · 2025-10-30T17:16:45Z

src/librustdoc/html/highlight.rs

+    /// Used for non-scraped code examples, ie source code pages and code examples in documentation.
+    Normal(u32),
+    /// Code examples without line numbers.
+    #[allow(dead_code)]


as far as I can tell, code examples in documentation don't have line numbers (and if they did, we wouldn't want them to have line number anchors, otherwise multiple examples would end up with conflicting IDs)

the dead_code seems unnecessary

I'm guessing it's because the data of the Empty variant (i.e. the u32) is never read?

lolbinarycat · 2025-10-30T17:28:43Z

src/librustdoc/html/highlight.rs

+        if let Some(backline) = token_handler.handle_backline() {
+            token_handler.push_token_without_backline_check(
+                None,
+                Cow::Owned(backline.to_string()),


The fact that we're using Cow::Owned here didn't sit right to me, so I did some digging and found out push_token_without_backline_check has overly restrictive lifetimes. It should use text: Cow<'_, str>, not text: Cow<'a, str>.

Actually, I think it could even be just text: &str, since it doesn't look like the code ever takes advantage of the Owned case and instead just uses it as impl Display, but that would require more refactoring than just changing a lifetime

backline is a LineNumber, so we need to convert it to something compatible.

I mean, I think impl Display would also work? not sure the monomorphization would be worth it, could maybe experiment in a follow-up PR.

Regardless, the lifetime still is overly restrictive, and it's hard to see that as anything but a mistake.

Hum, might be worth trying in a follow-up.

yotamofek · 2025-10-30T18:43:38Z

src/librustdoc/html/highlight.rs

    /// used to generate links.
    href_context: Option<HrefContext<'a, 'tcx>>,
-    write_line_number: fn(u32) -> String,
+    write_line_number: fn(u32) -> LineNumber,


The enum is a nice touch, but maybe we can go one step further and make it into a data-less, C-like enum? So line_number: LineNumber or something, instead of holding an fn pointer.
Fn pointers probably aren't great for perf (I think they can't be inlined? unless the compiler/LLVM are smart enough to make it into a sort-of enum?)
And also, might make code slightly nicer.

Good idea. Gonna implement that.

yotamofek · 2025-10-30T18:44:07Z

src/librustdoc/html/highlight.rs

+    /// Used for non-scraped code examples, ie source code pages and code examples in documentation.
+    Normal(u32),
+    /// Code examples without line numbers.
+    #[allow(dead_code)]


I'm guessing it's because the data of the Empty variant (i.e. the u32) is never read?

rustbot · 2025-10-31T15:35:27Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

GuillaumeGomez · 2025-10-31T15:36:14Z

Simplified it even further thanks to both your feedback! Let's check perf now. =D

@bors try @rust-timer queue

Simplify code to generate line numbers in highlight

lolbinarycat · 2025-10-31T16:12:03Z

In favor of merging this and refactoring push_token_without_backline_check in a followup PR if @yotamofek don't have any more concerns.

yotamofek · 2025-10-31T16:28:59Z

src/librustdoc/html/highlight.rs

-    // https://developers.google.com/search/docs/crawling-indexing/robots-meta-tag#data-nosnippet-attr
-    // Do not show "1 2 3 4 5 ..." in web search results.
-    format!("<a href=#{line} id={line} data-nosnippet>{line}</a>")
+/// Represents line numbers to be generated as HTML.


nit:

Suggested change

/// Represents line numbers to be generated as HTML.

/// Represents the type of line number to be generated as HTML.

maybe?

Agreed, adding it. :)

yotamofek · 2025-10-31T16:29:24Z

src/librustdoc/html/highlight.rs

    /// used to generate links.
    href_context: Option<HrefContext<'a, 'tcx>>,
-    write_line_number: fn(u32) -> String,
+    write_line_number: LineNumberKind,


Suggested change

write_line_number: LineNumberKind,

line_number_kind: LineNumberKind,

yotamofek · 2025-10-31T16:30:30Z

In favor of merging this and refactoring push_token_without_backline_check in a followup PR if @yotamofek don't have any more concerns.

Apart from two super-small nits, I think this should be merged :)

GuillaumeGomez · 2025-10-31T16:33:25Z

Let's wait for perf first then. :)

rust-bors · 2025-10-31T17:48:04Z

☀️ Try build successful (CI)
Build commit: d3e852c (d3e852cfcc15185557dd95819dd6ced0d5d965de, parent: 51f5892019f8fb07864647d46c4eb577d3b0719f)

rust-timer · 2025-10-31T19:40:58Z

Finished benchmarking commit (d3e852c): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.3%, -0.1%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary -2.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.0%	[-2.0%, -2.0%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 5.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.5%	[5.5%, 5.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

Results (secondary 0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Bootstrap: 474.501s -> 475.417s (0.19%)
Artifact size: 390.85 MiB -> 390.89 MiB (0.01%)

yotamofek · 2025-10-31T19:47:11Z

Filtered for only "doc" profile runs:
https://perf.rust-lang.org/compare.html?start=34a8c7368c84fc699fc83a8851a02f93fd655931&end=b384be56b050599ec6a78499b3413da442fb9150&stat=instructions%3Au&check=false&debug=false&opt=false
https://perf.rust-lang.org/compare.html?start=51f5892019f8fb07864647d46c4eb577d3b0719f&end=d3e852cfcc15185557dd95819dd6ced0d5d965de&stat=instructions%3Au&check=false&debug=false&opt=false
Newer run is even slightly greener 😎

GuillaumeGomez · 2025-10-31T22:46:15Z

Nice! Thanks to both of you. :)

@bors r=yotamofek,lolbinarycat rollup

bors · 2025-10-31T22:46:18Z

📌 Commit 5fabd2d has been approved by yotamofek,lolbinarycat

It is now in the queue for this repository.

…ht, r=yotamofek,lolbinarycat Simplify code to generate line numbers in highlight Follow-up of rust-lang#146992. cc `@yotamofek` r? `@lolbinarycat`

…ht, r=yotamofek,lolbinarycat Simplify code to generate line numbers in highlight Follow-up of rust-lang#146992. cc ``@yotamofek`` r? ``@lolbinarycat``

…ht, r=yotamofek,lolbinarycat Simplify code to generate line numbers in highlight Follow-up of rust-lang#146992. cc ```@yotamofek``` r? ```@lolbinarycat```

…ht, r=yotamofek,lolbinarycat Simplify code to generate line numbers in highlight Follow-up of rust-lang#146992. cc ````@yotamofek```` r? ````@lolbinarycat````

Rollup of 10 pull requests Successful merges: - #135602 (Tweak output of missing lifetime on associated type) - #139751 (Implement pin-project in pattern matching for `&pin mut|const T`) - #142682 (Update bundled musl to 1.2.5) - #148171 (Simplify code to generate line numbers in highlight) - #148263 (Unpin `libc` and `rustix` in `compiler` and `rustbook`) - #148301 ([rustdoc search] Include extern crates when filtering on `import`) - #148330 (Don't require dlltool with the dummy backend on MinGW) - #148338 (cleanup: upstream dropped amx-transpose functionality) - #148340 (Clippy subtree update) - #148343 (`nonpoison::Condvar` should take `MutexGuard` by reference) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #148171 - GuillaumeGomez:line-number-highlight, r=yotamofek,lolbinarycat Simplify code to generate line numbers in highlight Follow-up of #146992. cc `````@yotamofek````` r? `````@lolbinarycat`````

rustbot assigned lolbinarycat Oct 27, 2025

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Oct 27, 2025

Auto merge of #148171 - GuillaumeGomez:line-number-highlight, r=<try>

b384be5

Simplify code to generate line numbers in highlight

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 27, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 27, 2025

lolbinarycat reviewed Oct 30, 2025

View reviewed changes

yotamofek reviewed Oct 30, 2025

View reviewed changes

Simplify code to generate line numbers in highlight

04a10f7

GuillaumeGomez force-pushed the line-number-highlight branch from db4815c to 04a10f7 Compare October 31, 2025 15:35

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Oct 31, 2025

Auto merge of #148171 - GuillaumeGomez:line-number-highlight, r=<try>

d3e852c

Simplify code to generate line numbers in highlight

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 31, 2025

yotamofek reviewed Oct 31, 2025

View reviewed changes

Improve code

5fabd2d

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 31, 2025

yotamofek approved these changes Oct 31, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 31, 2025

Zalathar mentioned this pull request Nov 1, 2025

Rollup of 19 pull requests #148351

Closed

Zalathar mentioned this pull request Nov 1, 2025

Rollup of 19 pull requests #148354

Closed

matthiaskrgr mentioned this pull request Nov 1, 2025

Rollup of 11 pull requests #148355

Closed

matthiaskrgr mentioned this pull request Nov 1, 2025

Rollup of 10 pull requests #148356

Merged

bors merged commit efcf705 into rust-lang:master Nov 1, 2025
11 checks passed

rustbot added this to the 1.93.0 milestone Nov 1, 2025

GuillaumeGomez deleted the line-number-highlight branch November 3, 2025 10:51

	/// Represents line numbers to be generated as HTML.
	/// Represents the type of line number to be generated as HTML.

	write_line_number: LineNumberKind,
	line_number_kind: LineNumberKind,

Simplify code to generate line numbers in highlight #148171

Simplify code to generate line numbers in highlight #148171

Uh oh!

Conversation

GuillaumeGomez commented Oct 27, 2025

Uh oh!

GuillaumeGomez commented Oct 27, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors bot commented Oct 27, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Oct 27, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

GuillaumeGomez commented Oct 30, 2025

Uh oh!

lolbinarycat commented Oct 30, 2025

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Oct 31, 2025

Uh oh!

GuillaumeGomez commented Oct 31, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

lolbinarycat commented Oct 31, 2025

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!

yotamofek commented Oct 31, 2025

Uh oh!

GuillaumeGomez commented Oct 31, 2025

Uh oh!

rust-bors bot commented Oct 31, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Oct 31, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

yotamofek commented Oct 31, 2025

Uh oh!

GuillaumeGomez commented Oct 31, 2025

Uh oh!

bors commented Oct 31, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!