refactor: replace InternedString with Cow in IndexPackage

[0xPoe]

What does this PR try to resolve?

ref #14834

As described in the issue, we want to move IndexPackage into cargo-util-schemas. However, it contains InternedString fields, which we don't want to expose as part of the public API.
This PR replaces InternedString with Cow.

And also, as @weihanglo's suggested, I implemented the From/Into trait to simplify code.

From Cow tarit implementation: 42f593f (#15559)

Replace InternedString with into() across the whole codebase: 5f90098 (#15559)
I am unsure if it worsens the readability. Feel free to comment.

How should we test and review this PR?

It shouldn't change or break any tests.

Benchmark 1: #15559 (comment)
Benchmark 2: #15559 (comment)

Additional information

None

[rustbot]

r? @ehuss

rustbot has assigned @ehuss.
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

[ehuss]

I would recommend at least doing some benchmarking using our bench suite. https://github.com/rust-lang/cargo/tree/master/benches contains some documentation, and particularly the resolve bench. cargo bench -p benchsuite --bench resolve is a good place to start (while you are figuring out how to run the benchmarks you can do an individual workspace like resolve_ws/rust to speed things up), and Comparing implementation explains how you can do a comparison report against master.

[weihanglo]

I wonder if we could change InternString::from_cow to something like:

impl<'a> From<Cow<'a, str>> for InternedString {
    fn from(cs: Cow<'a, str>) -> Self {
        let mut cache = interned_storage();
        let s = cache.get(cs.as_ref()).copied().unwrap_or_else(|| {
            let s = cs.into_owned().leak();
            cache.insert(s);
            s
        });

        InternedString { inner: s }
    }
}

So in other places we can rely on Into<InternedString>.

[0xPoe]

@weihanglo Do you think it makes sense to replace all instances of InternedString::new with .into() where possible? There are quite a few usages of InternedString::new.

[weihanglo]

I am fine with wherever reasonable. Could also leave them for future

[weihanglo]

I am fine with wherever reasonable. Could also leave them for future

[0xPoe]

I will update all of them in a separate commit.

[0xPoe]

I have done it in this commit. 5f90098 (#15559).
I am unsure if it worsens the readability. Maybe we shouldn't do it at all 😄

[weihanglo]

I am fine with either. Looks like this is ready for review/merge?

[0xPoe]

Yes. It is ready for review. Thank you!

[0xPoe]

I’ve tested this change several times using the resolve_ws/rust benchmark suite.

My test env:

  Model Name:	MacBook Pro
  Chip:	Apple M4 Max
  Total Number of Cores:	14 (10 performance and 4 efficiency)
  Memory:	36 GB

Test result:
master commit: HEAD is now at c2c636a fix(toml): Remove workaround for rustc frontmatter support (#15570)
master: cargo bench -p benchsuite --bench resolve -- resolve_ws/rust --save-baseline master --measurement-time 50 --sample-size 500
patch: cargo bench -p benchsuite --bench resolve -- resolve_ws/rust --baseline master --measurement-time 50 --sample-size 500

First round:

Benchmark	Time (mean)	Change	Outliers
resolve_ws/rust	91.910 ms	-0.9313%	31 (6.20%) - 14 high mild, 16 high severe
resolve_ws/rust-ws-inherit	92.021 ms	+0.7503%	43 (8.60%) - 14 high mild, 22 high severe

Second round:

Benchmark	Time (mean)	Change	Outliers
resolve_ws/rust	92.142 ms	+1.7303%	33 (6.60%) - 10 high mild, 23 high severe
resolve_ws/rust-ws-inherit	91.193 ms	-0.0707%	30 (6.00%) - 8 high mild, 22 high severe

Third round:

Benchmark	Time (mean)	Change	Outliers
resolve_ws/rust	91.398 ms	+1.2693%	28 (5.60%) - 8 high mild, 20 high severe
resolve_ws/rust-ws-inherit	90.558 ms	+0.5349%	26 (5.20%) - 8 high mild, 18 high severe

Fourth round:

Benchmark	Time (mean)	Change	Outliers
resolve_ws/rust	91.193 ms	+0.8302%	26 (5.20%) - 11 high mild, 15 high severe
resolve_ws/rust-ws-inherit	90.591 ms	+0.8267%	34 (6.80%) - 6 high mild, 17 high severe

[0xPoe]

I’ve tested this change several times using the resolve_ws/tikv benchmark suite.

My test env:

  Model Name:	MacBook Pro
  Chip:	Apple M4 Max
  Total Number of Cores:	14 (10 performance and 4 efficiency)
  Memory:	36 GB

Test result:
master commit: HEAD is now at c2c636a fix(toml): Remove workaround for rustc frontmatter support (#15570)
master: cargo bench -p benchsuite --bench resolve -- resolve_ws/tikv --save-baseline master --measurement-time 100 --sample-size 600
patch: cargo bench -p benchsuite --bench resolve -- resolve_ws/tikv --baseline master --measurement-time 100 --sample-size 600

First round:

Benchmark	Time (mean)	Change	Outliers
resolve_ws/tikv	131.32 ms	-2.4203%	42 (7.00%) - 18 high mild, 24 high severe

Second round:

Benchmark	Time (mean)	Change	Outliers
resolve_ws/tikv	133.14 ms	+0.8323%	37 (6.17%) - 14 high mild, 23 high severe

Third round:

Benchmark	Time (mean)	Change	Outliers
resolve_ws/tikv	134.06 ms	+0.9986%	30 (5.00%) - 10 high mild, 20 high severe

Fourth round:

Benchmark	Time (mean)	Change	Outliers
resolve_ws/tikv	135.27 ms	+2.3135%	106 (17.67%) - 71 high mild, 35 high severe

[0xPoe]

🔢 Self-check (PR reviewed by myself and ready for feedback)

• Code compiles successfully

• Benchmark Tests

• All tests pass

• PR title and description updated

• Documentation PR created (or confirmed not needed)

• PR size is reasonable

[weihanglo]

The difference shown in the benchmark seems insignificant, so going to merge this. Thank you so much!

[0xPoe]

Thanks for your review! 💚 💙 💜 💛 ❤️