Eliminate CrateMetadataRef.

[nnethercote]

There are a number of things I dislike about CrateMetadataRef.

• It contains two fields cstore and cdata. The latter points to data within the former. It's like having an Elem type that has a reference to a vec element and also a reference to the vec itself. Weird.
• The cdata field gets a lot of use, and the Deref impl just derefs that field. The cstore field is rarely used.
• CrateMetadataRef is not a good name.
• Variables named cdata sometimes refer to values of this type and sometimes to values of type CrateMetadata, which is confusing.

The good news is that CrateMetadataRef is not necessary and can be replaced with &CrateMetadata. Why? Everywhere that CrateMetadataRef is used, a TyCtxt is also present, and the CStore is accessible from the TyCtxt with CStore::from_tcx.

So this commit removes CrateMetadataRef and replaces all its uses with &CrateMetadata. Notes:

• This requires adding only two uses of CStore::from_tcx, which shows how rarely the cstore field was used.
• get_crate_data now matches get_crate_data_mut more closely.
• A few variables are renamed for consistency, e.g. data/cmeta -> cdata.
• An unnecessary local variable (local_cdata) in decode_expn_id is removed.
• All the CrateMetadataRef methods become CrateMetadata methods, and their receiver changes from self to &self.
• RawDefId::decode_from_cdata is inlined and removed, because it has a single call site.

r? @mejrs

[nnethercote]

Locally this was a perf win, mostly on doc builds. Let's try on CI:

@bors try @rust-timer queue

cc @petrochenkov

[rust-bors]

☀️ Try build successful (CI)
Build commit: 073731e (073731e7481a7399dd3febf125eb5483b67d4642, parent: 913e4bea83424658d76712fee9c52452a3a9ef0e)

[rust-timer]

Finished benchmarking commit (073731e): 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
@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.7%	[0.3%, 1.2%]	2
Improvements ✅ (primary)	-0.6%	[-1.3%, -0.3%]	14
Improvements ✅ (secondary)	-0.8%	[-1.3%, -0.2%]	30
All ❌✅ (primary)	-0.6%	[-1.3%, -0.3%]	14

Max RSS (memory usage)

Results (primary 1.8%)

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

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

Cycles

Results (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
Regressions ❌ (secondary)	2.2%	[2.2%, 2.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.2%	[-4.3%, -2.1%]	6
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: 492.161s -> 492.336s (0.04%)
Artifact size: 394.33 MiB -> 394.26 MiB (-0.02%)

[nnethercote]

Perf results are good, and match what I saw locally.

[mejrs]

This looks really nice, but I think petrochenkov has some thoughts :)

[petrochenkov]

Everywhere that CrateMetadataRef is used, a TyCtxt is also present, and the CStore is accessible from the TyCtxt with CStore::from_tcx

This wasn't the case until recently, only Session was passed around, not full tcx, because no other parts of tcx were used during decoding, that's why cstore had to be preserved in CrateMetadataRef.
Also if CStrore::from_tcx was used often, it could be a pessimization compared to passing cstore around, because it's going through a lock, but fortunately CStore::from_tcx is not used often.

[petrochenkov]

@bors r=mejrs,petrochenkov

[rust-bors]

📌 Commit f1f655d has been approved by mejrs,petrochenkov

It is now in the queue for this repository.