Open Bug 1720049 Opened 3 years ago Updated 3 years ago

Consider changing Variant tag type from uint_fast8_t to user's choice, or more compact type if possible

Categories

(Core :: MFBT, task)

task

Tracking

()

People

(Reporter: mozbugz, Unassigned)

References

Details

Spawned from Emilio's review comment:

I wonder if we should just use uint8_t directly to get the same size across platforms.

Context: Bug 1719959 was a bug showing a build failure on OpenBSD/i386 because of a static_assert that the Variant tag type (uint_fast8_t in this case) should be smaller than size_t, but this may not be true on all platforms!
I chose to fix the test there, but keep the uint_fast8_t.

Though Emilio probably thought about making things simpler and more consistent by using the non-fast uint8_t, and I argued for keeping the "fast" one as originally intended (and not wanting to experiment in a quick bug-fixing patch), that made me think about another aspect: The size of whole Variant objects, and whether picking a smaller tag in some situations may help?

Some early thoughts:
The Variant type is roughly something like this:

template <typename... Ts>
struct { Tag mTag; union { Ts...; } mStorage; };

I would guess that in most situations, the Variant alternatives (the Ts) would be at least 32-64 bits (numbers) or bigger (classes), with similar common alignment, in which case the tag size could be 32 or 64 without impacting the total size.

But maybe there are some cases where the alternatives would be small enough, that choosing a smaller tag type would help?
Also we could let the user pick their preferred tag type.

I think a first useful step would be to survey what's actually used in Firefox.
Also it would be good to benchmark whether changing the tag type would have real performance impact, and whether compacting the tag + storage could help as well (in the ideal world, a Variant could be small+trivial enough that it could be passed through registers).

To be investigated further...

You need to log in before you can comment on or make changes to this bug.