core: use FormattingOptions by reference

[joboet]

Most of the time, the options passed to Formatter will be the unmodified default options. The current approach of using the options by-value means that a new FormattingOptions structure has to be created every time a formatter is constructed. With this PR, Formatter stores the options by-reference, which means that one FormattingOptions instance in static memory is enough to create a Formatter. In the future (and in particular once #119899 has merged), we might even consider removing fmt::rt::Placeholder in favour of FormattingOptions, which would – in combination with this change – decrease the option-copying even further.

CC #118117 @EliasHolzmann

[rustbot]

r? @jhpratt

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

[joboet]

@bors try @rust-timer queue

[bors]

⌛ Trying commit cffe20d with merge 3bd291a...

[bors]

☀️ Try build successful - checks-actions
Build commit: 3bd291a (3bd291aa211330a3e52fe2846309b9aac4285e5b)

[joshtriplett]

Paging @m-ou-se for expertise on the formatting system.

Also, perf doesn't seem likely to be sufficient. @m-ou-se, thoughts on measuring this better?

[rust-timer]

Finished benchmarking commit (3bd291a): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.2%, 0.3%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[-0.4%, 0.3%]	3

Max RSS (memory usage)

Results (primary -0.9%, secondary -4.2%)

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)	4.1%	[1.0%, 10.5%]	5
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.1%	[-7.5%, -2.1%]	11
Improvements ✅ (secondary)	-4.2%	[-6.5%, -2.4%]	5
All ❌✅ (primary)	-0.9%	[-7.5%, 10.5%]	16

Cycles

Results (primary -14.5%, secondary -1.1%)

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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-14.5%	[-19.2%, -11.0%]	13
Improvements ✅ (secondary)	-1.1%	[-1.1%, -1.1%]	1
All ❌✅ (primary)	-14.5%	[-19.2%, -11.0%]	13

Binary size

Results (primary 0.0%, secondary 0.0%)

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)	0.1%	[0.0%, 0.3%]	43
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	15
Improvements ✅ (primary)	-0.1%	[-0.2%, -0.0%]	9
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[-0.2%, 0.3%]	52

Bootstrap: missing data
Artifact size: 348.27 MiB -> 348.23 MiB (-0.01%)

[m-ou-se]

If it makes any measurable difference, this seems fine.

However, ideally, we'd make FormattingOptions significantly smaller.

The problem is that FormattingOptions needs to contain width: Option<usize> and precision: Option<usize>, in case someone wants to pad their value to eighteen quintillion spaces and/or have eighteen quintillion digits of precision.

Perhaps we can all agree that it's fine to reduce it to an u32 or u16 or u8. If we then also reserve MAX for None, we can avoid wasting space on the Option. (Or move move the None flags into the existing flags field.)

That would technically be a breaking change, though.

[joshtriplett]

width and precision are returned by value, right? So moving None into the flags field would not be breaking?

Reducing the maximums would be, but suppose we made one of the flags "use the remaining fields as a pointer to a larger structure"? And then you couldn't use values bigger than u16 without an implicit allocation, which seems like a reasonable restriction.

[m-ou-se]

This is the information that FormattingOptions contains:

• The + flag (1 bit)
• The - flag (1 bit)
• The # flag (1 bit)
• The 0 flag (1 bit)
• The x? flag (1 bit)
• The X? flag (1 bit)
• The alignment (2 bits)
• The fill character (21 bits)
• Whether a width is specified (1 bit)
• Whether a precision is specified (1 bit)
• If used, the width (a full usize)
• If used, the precision (a full usize)

Everything except the last two can simply fit in a u32 (it's 31 bits in total).
If we can accept a max width and precision of u16::MAX, we can make a FormattingOptions that is exactly 64 bits in size; the same size as a thin reference on most platforms.

If we keep them as full usizes, we can still reduce the type to 3 usizes in size.

[m-ou-se]

Reducing the maximums would be, but suppose we made one of the flags "use the remaining fields as a pointer to a larger structure"? And then you couldn't use values bigger than u16 without an implicit allocation, which seems like a reasonable restriction.

Then we'd still need to make the structure two words in size rather than one, to be able to store the pointer somewhere. We could do some alignment trickery to steal one or two bits of the pointer, and move all the flags into the allocated part to keep FormatArgs one pointer in size, but then every single thing that checks formatting flags will get branches to check whether to follow a pointer or not. And we'll need to deal with allocation somehow, even in no_std. I don't think we should put this much effort into something that is probably never used.

[joshtriplett]

We could do some alignment trickery to steal one or two bits of the pointer, and move all the flags into the allocated part to keep FormatArgs one pointer in size, but then every single thing that check formatting flags will get branches to check whether to follow a pointer or not. I don't think we should pay that cost for something that is probably never used.

Checking a single bit doesn't seem that awful, if everything else along that branch is a cold path. I expect we would probably win on net by making the structure a u64. (But as always, benchmarks speak louder than words.)

[m-ou-se]

Note that all of this also needs to work in no_std situations, where we cannot allocate or anything like that. And we should pay some attention to binary size too.

[joshtriplett]

Note that all of this also needs to work in no_std situations, where we cannot allocate or anything like that.

I'm up for trying to apply the restriction to u16 across the board. This was an idea for a fallback if that restriction breaks something: we could say that if you have alloc you can use more than u16 and otherwise you can't.

[m-ou-se]

if you have alloc

That doesn't make much sense. We don't even have a mechanism for that.

I'll do a crater run for the reduction to u16.

[m-ou-se]

Running the experiment here: #136932

[Amanieu]

r? @m-ou-se

[joboet]

This is the information that FormattingOptions contains:

* The `+` flag (1 bit)

* The `-` flag (1 bit)

* The `#` flag (1 bit)

* The `0` flag (1 bit)

* The `x?` flag (1 bit)

* The `X?` flag (1 bit)

* The alignment (2 bits)

* The fill character (21 bits)

* Whether a width is specified (1 bit)

* Whether a precision is specified (1 bit)

* If used, the width (a full usize)

* If used, the precision (a full usize)

Everything except the last two can simply fit in a u32 (it's 31 bits in total). If we can accept a max width and precision of u16::MAX, we can make a FormattingOptions that is exactly 64 bits in size; the same size as a thin reference on most platforms.

If we keep them as full usizes, we can still reduce the type to 3 usizes in size.

I much prefer these approaches, so I'm closing this.

[EliasHolzmann]

Thanks for the mention @joboet! And sorry for me somehow completely missing your point in #118117 (comment) :)

Regarding the discussion on making FormattingOptions smaller, there was a bit of conversation about this topic in the original implementation PR for FormattingOptions, too: #118159 (comment) (and following comments). However, I believe that the approach suggested here is pretty much equivalent to the final solution @programmerjake proposed in the other PR, which seems optimal to me.