[panstromek]

View all comments

Similar idea as #162535 and previous attempts.

Again, I also noticed that no test fails when I mess up the string, so I added a test case.

[panstromek]

perf test for field names of large structs. This probably won't be as visible, there's not that many of those. I'll try to pick only benchmarks that that have those + 2 other ones as a spot check.

@bors try
@rust-timer queue benchmarks=cargo-0.87.1,nalgebra-0.33.0,nalgebra-0.33.0-new-solver,eza-0.21.2,regex-automata-0.4.8,cranelift-codegen-0.119.0,hyper-1.6.0,image-0.25.6,ripgrep-14.1.1,ripgrep-14.1.1-nll,diesel-2.2.10,diesel-2.2.10-new-solver,tokio-1.53.1,await-call-tree,bitmaps-3.2.1

[rust-bors]

☀️ Try build successful (CI)
Build commit: 455d3ee (455d3ee7337cbd5399396321c912ae289397f52e)
Base parent: 2b548e4 (2b548e49fcdf52bed4446b3e245563681f5bc45d)

[rust-timer]

Finished benchmarking commit (455d3ee): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@bors rollup=never rustc-perf
@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.2%	[-0.4%, -0.1%]	16
Improvements ✅ (secondary)	-0.3%	[-0.5%, -0.2%]	5
All ❌✅ (primary)	-0.2%	[-0.4%, -0.1%]	16

Max RSS (memory usage)

Results (primary -0.7%, secondary -2.4%)

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

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.4%, 0.6%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.3%	[-2.0%, -0.6%]	7
Improvements ✅ (secondary)	-2.4%	[-2.4%, -2.4%]	1
All ❌✅ (primary)	-0.7%	[-2.0%, 0.6%]	11

Cycles

Results (primary -0.7%)

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

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.5%, 0.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.9%	[-2.6%, -1.2%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.7%	[-2.6%, 0.6%]	4

Binary size

Results (primary -0.1%)

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

	mean	range	count
Regressions ❌ (primary)	0.4%	[0.0%, 0.8%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.2%	[-0.4%, -0.0%]	22
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.1%	[-0.4%, 0.8%]	24

Bootstrap: missing data
Artifact size: 406.27 MiB -> 406.47 MiB (0.05%)

[panstromek]

Nice, looks better than I expected. This makes sense, the best result is ripgrep, which has two structs with 68 and 74 fields. That's where this helps the most.

[panstromek]

r? nnethercote

Are you ok if I keep assigning you these things or should I let rustbot roll the dice? I've been assigning you almost everything lately, but that's just because I work on things you implemented and I try to pick reviewer based on who worked on the relevant code.

[rustbot]

Changes to the code generated for builtin derived traits.

cc @nnethercote

[nnethercote]

Nice idea! Can you do the same thing for debug_struct_field[2345]_finish?

No problem with the reviews. I always ask an appropriate person directly instead of letting the random choice happen.

[panstromek]

Can you do the same thing for debug_struct_field[2345]_finish?

Oh, that's another question I have. I was going to try that in a separate PR to separate the impact, do you prefer to do all of them at once?

[nnethercote]

It's such a closely related change, might as well do it here. Can be a separate commit if you want.

[panstromek]

Let's see how this works out for smaller structs. I'm a bit worried tbh. I think this should be better as far as the generated code goes, because the MIR will be smaller, but there's also more Symbol manipulation and interning, so it might not be so clear cut.

@bors try @rust-timer queue

Note: I went a bit further and concatenated struct name with the fields as well. This was simpler to do in the debug_struct_field[2345]_finish at first so I did it for array based one as well.

[rust-bors]

📌 Commit 2ec69fe has been approved by nnethercote

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 5. This pull request will be tested once the tree is reopened.

[rustbot]

This PR was rebased onto a different main 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.

[panstromek]

rebased

@rustbot ready

[nnethercote]

@bors r+

[rust-bors]

📌 Commit 4719ede has been approved by nnethercote

It is now in the queue for this repository.