parser: Cleanup LazyTokenStream and avoid some clones

[petrochenkov]

by using a named struct instead of a closure.

r? @Aaron1011

[petrochenkov]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit d0c63bc with merge 99e4ef7ba85e18068c4232297da28acab7db20f6...

[bors]

☀️ Try build successful - checks-actions
Build commit: 99e4ef7ba85e18068c4232297da28acab7db20f6 (99e4ef7ba85e18068c4232297da28acab7db20f6)

[rust-timer]

Queued 99e4ef7ba85e18068c4232297da28acab7db20f6 with parent ffe5288, future comparison URL.

[rust-timer]

Finished benchmarking try commit (99e4ef7ba85e18068c4232297da28acab7db20f6): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

[Aaron1011]

It looks like adding a Lrc didn't result in any performance impact.

@bors r+

[bors]

📌 Commit d0c63bc has been approved by Aaron1011

[Aaron1011]

@bors rollup-

[petrochenkov]

@Aaron1011
So, #78712 introduces one more case of converting a lazy token stream into a non-lazy one, and I started thinking about using an enum again.

The second reason for introducing an enum is that using None (of Option<LazyTokenStream>) in #77255 as both an unknown and as an empty stream feels sketchy, although I can't give an example where it would be incorrect.
Is this case intended for private visibilities specifically?

The problem is that the requirement sizeof(Option<LazyTokenStream>) <= 8 byte is significant for performance but very restrictive, if LazyTokenStream is made into enum, then you can't add even an empty variant without growing its size!

struct S1(Box<u8>);
struct S2(Option<Box<u8>>);
enum S3 { None, Some(Box<u8>) }
enum S4 { None, Empty, Some(Box<u8>) }
struct S5(Option<S3>);
struct S6(Option<S4>);

[src/main.rs:11] size_of::<S1>() = 8
[src/main.rs:12] size_of::<S2>() = 8
[src/main.rs:13] size_of::<S3>() = 8
[src/main.rs:14] size_of::<S4>() = 16
[src/main.rs:15] size_of::<S5>() = 16
[src/main.rs:16] size_of::<S6>() = 16

Perhaps if 16 byte is significantly better in terms of allocations and cloning, it will compensate... I need to check.

[Mark-Simulacrum]

Since Lazytokenstream is heap allocated, you might consider bit packing the discriminant (by increasing alignment slightly); there's some helpers available in rustc data structures to do so somewhat easily. Unfortunately it's unlikely to be an ergonomic thing as you definitely lose matching and such.

[petrochenkov]

Some statistics of token stream lengths in fn collect_tokens collected from bootstrapping rustc and stdlib:

  1  377069
  4   45481
  7   27001
 13   15311
 10   13782
 17   11456
  5   11309
  3   10065
  9    8041
  2    5961
  6    5295
 16    2672
  8    2079
 12    1692
 18    1679
 11    1349
  0    1300
 14    1243
 20    1003
 21     988

So, optimizing for 1-token streams is important, and optimizing for 0-token streams is not too important.