Normalize alloc-id in tests. #116767

cjgillot · 2023-10-15T17:38:11Z

AllocIds are globally numbered in a rustc invocation. This makes them very sensitive to changes unrelated to what is being tested. This commit normalizes them by renumbering, in order of appearance in the output.

The renumbering allows to keep the identity, that a simple allocN wouldn't. This is useful when we have memory dumps.

cc @saethlin
r? @oli-obk

rustbot · 2023-10-15T17:38:21Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

oli-obk · 2023-10-16T08:09:04Z

@bors r+ p=1 bitrotty

bors · 2023-10-16T08:09:06Z

📌 Commit a8d90ba has been approved by oli-obk

It is now in the queue for this repository.

bors · 2023-10-16T08:58:35Z

⌛ Testing commit a8d90ba with merge 1c5c949...

Normalize alloc-id in tests. AllocIds are globally numbered in a rustc invocation. This makes them very sensitive to changes unrelated to what is being tested. This commit normalizes them by renumbering, in order of appearance in the output. The renumbering allows to keep the identity, that a simple `allocN` wouldn't. This is useful when we have memory dumps. cc `@saethlin` r? `@oli-obk`

bors · 2023-10-16T09:20:29Z

💔 Test failed - checks-actions

bors · 2023-10-16T15:32:11Z

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

oli-obk · 2023-10-16T15:37:22Z

r=me after rebase and rebless

cjgillot · 2023-10-16T17:03:44Z

@bors r=oli-obk

bors · 2023-10-16T17:03:46Z

📌 Commit 02424e4 has been approved by oli-obk

It is now in the queue for this repository.

src/tools/compiletest/src/runtest.rs

RalfJung · 2023-10-16T18:27:29Z

Ah I didn't realize one PR is based on the other. Please mention that in the PR description. :)

RalfJung · 2023-10-16T18:28:51Z

src/tools/compiletest/src/runtest.rs

+        // depending on the exact compilation flags and host architecture. Meanwhile, we want
+        // to keep them numbered, to see if the same id appears multiple times.


Suggested change

// depending on the exact compilation flags and host architecture. Meanwhile, we want

// to keep them numbered, to see if the same id appears multiple times.

// depending on the exact compilation flags and host architecture. We want

// to normalize those differences away. Meanwhile, we want to keep

// the allocations numbered, to see if the same id appears multiple times.

// So we remap to deterministic numbers that only depend on the subset

// of allocations that actually appears in the output.

RalfJung · 2023-10-16T18:30:47Z

src/tools/compiletest/src/runtest.rs

+                    let index = caps.get(2).unwrap().as_str().to_string();
+                    let (index, _) = seen_allocs.insert_full(index);
+                    write!(&mut ret, "{index}").unwrap();
+                    // If we have a tail finishing with `─`, this means pretty-printing.


There could be 0 ─ but then a final ╼. That doesn't seem to be handled?
Also the number of ─ can vary, since it might be alloc9 vs alloc10. So you probably want to eat all the trailing ─*╼ and replace them by something deterministic.

And same for the leading ╾─*.

oli-obk · 2023-10-17T19:06:56Z

@bors r+

bors · 2023-10-17T19:06:58Z

📌 Commit 1f90d85 has been approved by oli-obk

It is now in the queue for this repository.

bors · 2023-10-17T20:46:57Z

⌛ Testing commit 1f90d85 with merge 09df610...

bors · 2023-10-17T22:40:23Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 09df610 to master...

rust-timer · 2023-10-18T00:01:10Z

Finished benchmarking commit (09df610): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

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

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: 627.092s -> 626.723s (-0.06%)
Artifact size: 305.13 MiB -> 305.15 MiB (0.01%)

57: Pull upstream master 2023 10 18 r=pietroalbini a=Veykril * rust-lang/rust#116505 * rust-lang/rust#116840 * rust-lang/rust#116767 * rust-lang/rust#116855 * rust-lang/rust#116827 * rust-lang/rust#116787 * rust-lang/rust#116719 * rust-lang/rust#116717 * rust-lang/rust#111072 * rust-lang/rust#116844 * rust-lang/rust#115577 * rust-lang/rust#116756 * rust-lang/rust#116518 Co-authored-by: Urgau <urgau@numericable.fr> Co-authored-by: Esteban Küber <esteban@kuber.com.ar> Co-authored-by: Deadbeef <ent3rm4n@gmail.com> Co-authored-by: Ralf Jung <post@ralfj.de> Co-authored-by: Camille GILLOT <gillot.camille@gmail.com> Co-authored-by: Celina G. Val <celinval@amazon.com> Co-authored-by: Nicholas Nethercote <n.nethercote@gmail.com> Co-authored-by: Arthur Lafrance <lafrancearthur@gmail.com> Co-authored-by: Nikolay Arhipov <n@arhipov.net> Co-authored-by: Nikita Popov <npopov@redhat.com> Co-authored-by: bors <bors@rust-lang.org>

rustbot assigned oli-obk Oct 15, 2023

This comment has been minimized.

Sign in to view

cjgillot force-pushed the alloc-normalize branch from aab070c to a8d90ba Compare October 15, 2023 18:04

saethlin mentioned this pull request Oct 15, 2023

Automatically enable cross-crate inlining for small functions #116505

Merged

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 16, 2023

This comment has been minimized.

Sign in to view

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

Normalize alloc-id in tests.

02424e4

cjgillot force-pushed the alloc-normalize branch from a8d90ba to 02424e4 Compare October 16, 2023 16:29

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 16, 2023

Motivate renumbering and avoid underflow.

c423305

cjgillot commented Oct 16, 2023

View reviewed changes

src/tools/compiletest/src/runtest.rs Show resolved Hide resolved

RalfJung reviewed Oct 16, 2023

View reviewed changes

cjgillot added 2 commits October 16, 2023 18:33

Elaborate comment.

669b99d

Rebless.

0eb958a

This comment has been minimized.

Sign in to view

Stop trying to preserve pretty-printing.

1f90d85

cjgillot force-pushed the alloc-normalize branch from 42e3bd2 to 1f90d85 Compare October 16, 2023 19:15

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

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 17, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 17, 2023

bors merged commit 09df610 into rust-lang:master Oct 17, 2023
12 checks passed

rustbot added this to the 1.75.0 milestone Oct 17, 2023

Veykril mentioned this pull request Oct 18, 2023

Pull upstream master 2023 10 18 ferrocene/ferrocene#57

Merged

cjgillot deleted the alloc-normalize branch October 18, 2023 06:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normalize alloc-id in tests. #116767

Normalize alloc-id in tests. #116767

cjgillot commented Oct 15, 2023

rustbot commented Oct 15, 2023

This comment has been minimized.

oli-obk commented Oct 16, 2023

bors commented Oct 16, 2023

bors commented Oct 16, 2023

This comment has been minimized.

bors commented Oct 16, 2023

bors commented Oct 16, 2023

oli-obk commented Oct 16, 2023

cjgillot commented Oct 16, 2023

bors commented Oct 16, 2023

RalfJung commented Oct 16, 2023

RalfJung Oct 16, 2023

RalfJung Oct 16, 2023 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

oli-obk commented Oct 17, 2023

bors commented Oct 17, 2023

bors commented Oct 17, 2023

bors commented Oct 17, 2023

rust-timer commented Oct 18, 2023

		// depending on the exact compilation flags and host architecture. Meanwhile, we want
		// to keep them numbered, to see if the same id appears multiple times.

-        // depending on the exact compilation flags and host architecture. Meanwhile, we want
-        // to keep them numbered, to see if the same id appears multiple times.
+        // depending on the exact compilation flags and host architecture. We want
+        // to normalize those differences away. Meanwhile, we want to keep
+        // the allocations numbered, to see if the same id appears multiple times.
+        // So we remap to deterministic numbers that only depend on the subset
+        // of allocations that actually appears in the output.

Normalize alloc-id in tests. #116767

Normalize alloc-id in tests. #116767

Conversation

cjgillot commented Oct 15, 2023

rustbot commented Oct 15, 2023

This comment has been minimized.

oli-obk commented Oct 16, 2023

bors commented Oct 16, 2023

bors commented Oct 16, 2023

This comment has been minimized.

bors commented Oct 16, 2023

bors commented Oct 16, 2023

oli-obk commented Oct 16, 2023

cjgillot commented Oct 16, 2023

bors commented Oct 16, 2023

RalfJung commented Oct 16, 2023

RalfJung Oct 16, 2023

Choose a reason for hiding this comment

RalfJung Oct 16, 2023 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

oli-obk commented Oct 17, 2023

bors commented Oct 17, 2023

bors commented Oct 17, 2023

bors commented Oct 17, 2023

rust-timer commented Oct 18, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

RalfJung Oct 16, 2023 •

edited

Loading