Skip to content

QuantityFormatter: Take UnitAbbreviationsCache instance, Format with TQuantity #1551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 19, 2025

Conversation

lipchev
Copy link
Collaborator

@lipchev lipchev commented Apr 19, 2025

  • QuantityFormatter: introducing an UnitAbbreviationsCache instance and Format-ing using a generic TQuantity
  • UnitsNetSetup: introduced an instance property for the UnitFormatter
  • IQuantity: added the UnitKey property (implemented explicitly)
  • replaced the existing usages of the QuantityFormatter and marked the static Format overloads as [Obsolete]

fixes #1447

… Formating using a generic `TQuantity`

- `UnitsNetSetup`: introduced an instance property for the `UnitFormatter`
- `IQuantity`: added the UnitKey property (implemented explicitly)
- replaced the existing usages of the `QuantityFormatter` and marked the static `Format` overloads as `[Obsolete]`
@lipchev
Copy link
Collaborator Author

lipchev commented Apr 19, 2025

If you're ok with the addition of the UnitKey to the IQuantity interface (as a public property), then we could compensate it by removing the string ToString(IFormatProvider? provider) :

    public static string ToString<TQuantity>(this TQuantity quantity, IFormatProvider? formatProvider)
        where TQuantity : IQuantity, IFormattable
    {
        return quantity.ToString(null, formatProvider);
    }

Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a few minor suggestions.

}

private static string ToStringWithSignificantDigitsAfterRadix<TUnitType>(IQuantity<TUnitType> quantity, IFormatProvider formatProvider, int number)
where TUnitType : struct, Enum
private string ToStringWithSignificantDigitsAfterRadix<TQuantity>(TQuantity quantity, IFormatProvider formatProvider, int number)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use generics TQuantity several places in QuantityFormatter?

It does not seem used for anything or provide anything beyond what a regular IQuantity parameter provides.
The JIT has to create a copy of each generic class or class member, for each concrete type you use it with at runtime.

Usually, generics are used to preserve type info for outputs, but the string formatter only outputs strings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to avoid boxing the quantity using the IQuantity interface.

I'm aware of the (very small increase) that the generic method generates, but here we're already offsetting this by removing another generic method down the line (on the UnitFormatter).

I originally monitored the size after every additional method, but I didn't bother recording it here now - it's very small.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, the whole QuantityFormatter can be re-written without the use of any generics (by using something like double value, UnitKey unit, string format for the parameters).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could just pass the concrete values instead then, or even create a separate struct for value+UnitKey if we feel it helps.

I don't know, it just feels overkill to tack generics to everything just to avoid boxing of quantities by interface.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly, now that I think about it- the reason for having a quantity as the format parameter in the first place was that it was necessary for those other formats that we removed (tapping into the QuantityInfo).

I think this might actually be a worthwhile change to make (should I do it?).

On the question of boxing the structs- well that's probably the main reason microsoft went crazy with the INumber interfaces 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, perhaps this is a good place to note the possibility to implement certain things on the interface itself (and the caveats). Consider this example:

#if NET
        internal static abstract TQuantity Create(QuantityValue value, UnitKey unit); // on an intermediate interface extending IQuantity
#else

#if NET7_0_OR_GREATER
        /// <summary>
        ///     Creates an instance of the quantity from a specified value and unit.
        /// </summary>
        /// <param name="value">The numerical value of the quantity.</param>
        /// <param name="unit">The unit of the quantity.</param>
        /// <returns>An instance of the quantity with the specified value and unit.</returns>
        static abstract TSelf From(QuantityValue value, TUnitType unit);

        static TSelf IQuantityInstance<TSelf>.Create(QuantityValue value, UnitKey unit) => TSelf.From(value, unit.ToUnit<TUnitType>());
#endif

The only thing that the Mass (or the HowMuch) needs to implement is the static From(QuantityValue, MassUnit) method (which it already does) and now we can have an extension method that takes just a single type parameter:

    internal static TQuantity ArithmeticMean<TQuantity>(this IEnumerable<TQuantity> quantities)
        where TQuantity : IQuantityInstance<TQuantity>

And now the resulting TQuantity can be created directly using:

return TQuantity.Create(sumOfValues / nbValues, resultUnit); // resultUnit is of type `UnitKey`

Since we're talking about static methods, there isn't any boxing and we get 0 allocations.

However, this would not be the case if we were to use a default implementation of an instance method such as ToUnit(TUnit) (this is counterintuitive, and may change in a future .NET versions but currently this involves an interface dispatch or something, that ultimately makes the way like this: Mass -> IQuantity<Mass, MassUnit> -> Mass).

That's less of an issue when the return type is already an interface, like with the obsolete versions of these methods:

        [Obsolete("This method will be removed from the interface in the next major update. Consider using the UnitConverter.Default.ConvertTo(quantity, unit) method instead.")]
        IQuantity ToUnit(Enum unit)
#if NET
            => UnitConverter.Default.ConvertTo(this, unit)
#endif
        ;

Copy link
Collaborator Author

@lipchev lipchev Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly, now that I think about it- the reason for having a quantity as the format parameter in the first place was that it was necessary for those other formats that we removed (tapping into the QuantityInfo).

I think this might actually be a worthwhile change to make (should I do it?).

Actually, after thinking about a little more- although totally feasible, I don't think we should do it:

  1. Using the UnitKey as a parameter is exposing the internals and should generally be used internally (or for people who are forced to use loosely typed stuff).
  2. While the Mass.ToString itself wouldn't change, if we wanted to use QuantityFormatter.Default.Format(mass.Value, mass.UnitKey) (or another instance) we couldn't have the UnitKey implementation as explicit.

PS For the longest time, I had the UnitKey type as internal, frankly I can't remember what drove me to exposing it (might have been just so that HowMuch can benefit from the optimizations regarding the loosely-typed operations).

@angularsen
Copy link
Owner

If you're ok with the addition of the UnitKey to the IQuantity interface (as a public property), then we could compensate it by removing the string ToString(IFormatProvider? provider) :

    public static string ToString<TQuantity>(this TQuantity quantity, IFormatProvider? formatProvider)
        where TQuantity : IQuantity, IFormattable
    {
        return quantity.ToString(null, formatProvider);
    }

This feels a bit unnecessary, UnitKey and Unit duplicates the same information, and as far as I can tell we use UnitKey primarily for internal optimization purposes.

The implementation for UnitKey property also just calls UnitKey.ForUnit(Unit) anyway, so why don't we just call UnitKey.ForUnit(myQuantity.Unit) whenever we need the key internally?

Or have an extension method to do myQuantity.GetUnitKey() for shorter syntax.

@angularsen
Copy link
Owner

angularsen commented Apr 19, 2025

A couple of comments on UnitKey type:

  1. The ctor(Type, int) should
    2. Throw if passing non-enum Type
    3. Throw if !Enum.IsDefined(UnitType, UnitValue)
    4. ..or make the ctor private so you can only construct via generic factory methods, that at least checks the type is an enum, but IsDefined check is probably useful to have for those call paths too
    5. Parameters should have xmldoc, in particular it is not described that UnitType should be an enum type and that UnitValue is the enum's integer value
  2. Rename UnitType to UnitEnumType or EnumType
  3. Rename UnitValue to UnitEnumValue or EnumValue

@lipchev
Copy link
Collaborator Author

lipchev commented Apr 19, 2025

This feels a bit unnecessary, UnitKey and Unit duplicates the same information, and as far as I can tell we use UnitKey primarily for internal optimization purposes.

The implementation for UnitKey property also just calls UnitKey.ForUnit(Unit) anyway, so why don't we just call UnitKey.ForUnit(myQuantity.Unit) whenever we need the key internally?

Or have an extension method to do myQuantity.GetUnitKey() for shorter syntax.

The role of the UnitKey is as a replacement for the Enum Unit property of the IQuantity interface - it's use is redundant only when the TUnit is known. However, if we wanted to use TQuantity with a generic TUnit we'd have to use 2 type parameters; <TQuantity, TUnit>, but now the same methods can only have the TQuantity parameter, without having to go though boxing and unboxing the Enum.

@lipchev
Copy link
Collaborator Author

lipchev commented Apr 19, 2025

A couple of comments on UnitKey type:

1. The `ctor(Type, int)` should
   2. Throw if passing non-enum `Type`
   3. Throw if `!Enum.IsDefined(UnitType, UnitValue)`
   4. ..or make the ctor private so you can only construct via generic factory methods, that at least checks the type is an enum, but `IsDefined` check is probably useful to have for those call paths too
   5. Parameters should have xmldoc, in particular it is not described that `UnitType` should be an enum type and that `UnitValue` is the enum's integer value

I'll add another Create method and make the constructor internal, however I'm not sure about the existence check - I was kinda counting on it for the extending with custom units part. It's a bit of a hacky way of extending the MassUnit, but it works.

2. Rename `UnitType` to `UnitEnumType` or `EnumType`

3. Rename `UnitValue` to `UnitEnumValue` or `EnumValue`

I used the same names as we typically use for these parameters: GetDefaultAbbreviation(Type unitType, int unitValue) - I hope to eventually replace all these with a single UnitKey parameter (less primitives).

If I had to rename it, I'd probably use the UntEnumType and UnitEnumValue but honestly I don't see much reason to do it - so if you want it renamed, you pick the name. 😛

Comment on lines 239 to 243
public string Format<TQuantity>(TQuantity quantity)
where TQuantity : IQuantity
{
return Format(quantity, null, null);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of this overload - I wasn't sure whether to have it as a separate overload or have the format of the other overload as optional: string? format = null?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default values instead of many overloads.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the same names as we typically use for these parameter

I know, but it really isn't obvious what unitType and unitValue means without having to read the xmldoc first. unitEnumType is more explicit, we don't accept any other types. I'll take readability over a bit longer names any day.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the same names as we typically use for these parameter

I know, but it really isn't obvious what unitType and unitValue means without having to read the xmldoc first. unitEnumType is more explicit, we don't accept any other types. I'll take readability over a bit longer names any day.

It's like asking me to rename my children but ok... fixed in 3d7a7c5

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added the Create method with an Enum check, but it might also make sense to actually use it (in the places where we currently have the primitives)..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added the Create method with an Enum check, but it might also make sense to actually use it (in the places where we currently have the primitives)..

fixed in 38e868a

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default values instead of many overloads.

fixed in e829882

Does this also apply for the methods of the IFormattable interface? I'd be happy to remove the overloads and simply make the parameters optional (there wouldn't be any need for the ToString extensions). Note we currently have just one of the overloads defined on the interface, the other one (ToString(string? format)) is just generated as a sort of an overload helper.

The problem is that removing them would be breaking change:

  1. for the HowMuch extensions: I reckon this is not gonna be missed
  2. for people using Mass.ToString(CultureInfo.InvariantCulture) : there is probably somebody using this, but I'm not sure how useful this is..

Copy link
Owner

@angularsen angularsen Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're already breaking changes all over, but generally I'd like existing code to "just work" when recompiled as much as possible.

I see IFormattable requires public string ToString(string? format, IFormatProvider? provider), and all objects already have public string ToString(), so maybe for ToString() we are a bit limited on how much we can use default values to avoid overloads.

Copy link
Collaborator Author

@lipchev lipchev Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see IFormattable requires public string ToString(string? format, IFormatProvider? provider), and all objects already have public string ToString(), so maybe for ToString() we are a bit limited on how much we can use default values to avoid overloads.

We can still make the nullable parameters optional in the override- it's still attached to the IFormattable interface.

We're already breaking changes all over, but generally I'd like existing code to "just work" when recompiled as much as possible.

Unfortunately, here this is exactly the tradeoff - if we want to not have a breaking change, we have to have both overloads (whether it's as extensions or members).

PS There isn't a way to have an [Obsolete] middle-ground (as far as I can tell)..

@angularsen
Copy link
Owner

angularsen commented Apr 19, 2025

However, if we wanted to use TQuantity with a generic TUnit we'd have to use 2 type parameters; <TQuantity, TUnit>, but now the same methods can only have the TQuantity parameter, without having to go though boxing and unboxing the Enum.

For my understanding, can you provide an example with and without UnitKey added to IQuantity, that illustrates why it is useful or needed?

@lipchev
Copy link
Collaborator Author

lipchev commented Apr 19, 2025

However, if we wanted to use TQuantity with a generic TUnit we'd have to use 2 type parameters; <TQuantity, TUnit>, but now the same methods can only have the TQuantity parameter, without having to go though boxing and unboxing the Enum.

For my understanding, can you provide an example with and without UnitKey added to IQuantity, that illustrates why it is useful or needed?

I think the QuantityFormatter was probably the best example, but let me try make a simplified example:

    internal string GetDefaultAbbreviation<TQuantity, TUnit>(TQuantity quantity)
        where TQuantity : IQuantity<TUnit>
        where TUnit : struct, Enum
    {
        return _unitAbbreviations.GetDefaultAbbreviation(quantity.Unit); // no boxing
    }

    internal string GetDefaultAbbreviationWithTypeParameter<TQuantity>(TQuantity quantity)
        where TQuantity : IQuantity
    {
        return _unitAbbreviations.GetDefaultAbbreviation(quantity.UnitKey); // no boxing
    }

Now imagine how we would call these two extensions with a concrete type:

        var a1 = GetDefaultAbbreviation<Mass, MassUnit>(Mass.Zero);
        var a2 = GetDefaultAbbreviationWithTypeParameter(Mass.Zero);

In the example of a1 there is no way for the second parameter to be inferred automatically (I have to type it manually).

Copy link
Collaborator Author

@lipchev lipchev Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benchmark results are out, but first a note on the size:

  • before: 2.18 MB (2 294 784 bytes)
  • after: 2.19 MB (2 301 440 bytes)

Before (NET8):

Method Job Runtime NbConversions Format Mean Error StdDev Ratio RatioSD Gen0 Allocated Alloc Ratio
MassToString .NET 8.0 .NET 8.0 1000 A 25.87 μs 0.475 μs 0.444 μs 1.00 0.02 1.8921 31.25 KB 1.00
VolumeFlowToString .NET 8.0 .NET 8.0 1000 A 26.10 μs 0.300 μs 0.250 μs 1.01 0.02 1.8921 31.25 KB 1.00
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 A 97.03 μs 0.278 μs 0.247 μs 1.00 0.00 10.1318 62.68 KB 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 A 97.87 μs 0.735 μs 0.688 μs 1.01 0.01 10.1318 62.68 KB 1.00
MassToString .NET 8.0 .NET 8.0 1000 E 132.45 μs 0.638 μs 0.597 μs 1.00 0.01 9.0332 149.52 KB 1.00
VolumeFlowToString .NET 8.0 .NET 8.0 1000 E 135.59 μs 0.611 μs 0.510 μs 1.02 0.01 9.5215 157.73 KB 1.05
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 E 503.20 μs 3.910 μs 3.657 μs 1.00 0.01 61.5234 383.86 KB 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 E 513.89 μs 4.447 μs 4.160 μs 1.02 0.01 62.5000 388.93 KB 1.01
MassToString .NET 8.0 .NET 8.0 1000 G 132.35 μs 0.903 μs 0.845 μs 1.00 0.01 8.5449 140.54 KB 1.00
VolumeFlowToString .NET 8.0 .NET 8.0 1000 G 139.97 μs 0.765 μs 0.678 μs 1.06 0.01 8.7891 145.59 KB 1.04
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 G 492.04 μs 1.356 μs 1.059 μs 1.00 0.00 57.1289 353.68 KB 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 G 498.79 μs 4.060 μs 3.798 μs 1.01 0.01 58.5938 361.93 KB 1.02
MassToString .NET 8.0 .NET 8.0 1000 N 143.85 μs 0.263 μs 0.233 μs 1.00 0.00 8.3008 135.62 KB 1.00
VolumeFlowToString .NET 8.0 .NET 8.0 1000 N 147.99 μs 0.259 μs 0.242 μs 1.03 0.00 8.5449 143.47 KB 1.06
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 N 513.30 μs 1.213 μs 1.075 μs 1.00 0.00 56.6406 353.35 KB 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 N 501.80 μs 3.087 μs 2.887 μs 0.98 0.01 57.6172 358.88 KB 1.02
MassToString .NET 8.0 .NET 8.0 1000 S 218.76 μs 1.542 μs 1.443 μs 1.00 0.01 20.0195 328.04 KB 1.00
VolumeFlowToString .NET 8.0 .NET 8.0 1000 S 219.61 μs 0.730 μs 0.647 μs 1.00 0.01 20.2637 333.09 KB 1.02
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 S 690.58 μs 2.153 μs 1.909 μs 1.00 0.00 95.7031 588.75 KB 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 S 662.66 μs 3.150 μs 2.947 μs 0.96 0.00 96.6797 596.99 KB 1.01

Before (NET9)

Method Job Runtime NbConversions Format Mean Error StdDev Ratio Gen0 Allocated Alloc Ratio
MassToString .NET 9.0 .NET 9.0 1000 A 31.94 μs 0.109 μs 0.102 μs 1.00 1.8921 31.25 KB 1.00
VolumeFlowToString .NET 9.0 .NET 9.0 1000 A 33.19 μs 0.200 μs 0.187 μs 1.04 1.8921 31.25 KB 1.00
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 A 92.59 μs 0.553 μs 0.517 μs 1.00 10.1318 62.68 KB 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 A 93.89 μs 0.719 μs 0.638 μs 1.01 10.1318 62.68 KB 1.00
MassToString .NET 9.0 .NET 9.0 1000 E 130.05 μs 0.100 μs 0.078 μs 1.00 9.0332 149.52 KB 1.00
VolumeFlowToString .NET 9.0 .NET 9.0 1000 E 133.25 μs 0.665 μs 0.622 μs 1.02 9.5215 157.73 KB 1.05
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 E 507.10 μs 3.242 μs 2.874 μs 1.00 61.5234 383.86 KB 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 E 499.04 μs 1.143 μs 1.070 μs 0.98 62.5000 388.93 KB 1.01
MassToString .NET 9.0 .NET 9.0 1000 G 128.11 μs 0.424 μs 0.397 μs 1.00 8.5449 140.54 KB 1.00
VolumeFlowToString .NET 9.0 .NET 9.0 1000 G 131.94 μs 0.567 μs 0.531 μs 1.03 8.7891 145.59 KB 1.04
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 G 487.63 μs 1.922 μs 1.798 μs 1.00 56.6406 353.68 KB 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 G 490.62 μs 2.421 μs 2.147 μs 1.01 58.5938 361.93 KB 1.02
MassToString .NET 9.0 .NET 9.0 1000 N 143.22 μs 0.812 μs 0.759 μs 1.00 8.3008 135.62 KB 1.00
VolumeFlowToString .NET 9.0 .NET 9.0 1000 N 144.14 μs 0.508 μs 0.450 μs 1.01 8.5449 143.47 KB 1.06
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 N 493.37 μs 1.085 μs 0.961 μs 1.00 56.6406 353.35 KB 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 N 494.64 μs 2.060 μs 1.826 μs 1.00 57.6172 358.88 KB 1.02
MassToString .NET 9.0 .NET 9.0 1000 S 161.52 μs 0.955 μs 0.893 μs 1.00 13.1836 218.66 KB 1.00
VolumeFlowToString .NET 9.0 .NET 9.0 1000 S 162.04 μs 0.825 μs 0.731 μs 1.00 13.6719 223.72 KB 1.02
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 S 657.28 μs 4.616 μs 4.318 μs 1.00 95.7031 588.75 KB 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 S 693.80 μs 6.590 μs 5.842 μs 1.06 96.6797 596.99 KB 1.01

After:

Method Job Runtime NbConversions Format Mean Error StdDev Median Ratio RatioSD Gen0 Allocated Alloc Ratio
MassToString .NET 9.0 .NET 9.0 1000 A 24.42 μs 0.062 μs 0.055 μs 24.42 μs 1.00 0.00 - - NA
VolumeFlowToString .NET 9.0 .NET 9.0 1000 A 26.33 μs 0.009 μs 0.009 μs 26.33 μs 1.08 0.00 - - NA
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 A 92.61 μs 0.223 μs 0.209 μs 92.61 μs 1.00 0.00 5.0049 32094 B 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 A 87.29 μs 0.058 μs 0.054 μs 87.29 μs 0.94 0.00 5.0049 32094 B 1.00
MassToString .NET 9.0 .NET 9.0 1000 E 101.32 μs 0.165 μs 0.146 μs 101.34 μs 1.00 0.00 6.2256 105104 B 1.00
VolumeFlowToString .NET 9.0 .NET 9.0 1000 E 105.06 μs 0.555 μs 0.463 μs 105.26 μs 1.04 0.00 6.7139 113520 B 1.08
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 E 399.10 μs 0.768 μs 0.681 μs 399.22 μs 1.00 0.00 23.9258 152362 B 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 E 394.08 μs 0.313 μs 0.244 μs 394.11 μs 0.99 0.00 24.9023 157554 B 1.03
MassToString .NET 9.0 .NET 9.0 1000 G 101.35 μs 0.199 μs 0.176 μs 101.27 μs 1.00 0.00 5.2490 87912 B 1.00
VolumeFlowToString .NET 9.0 .NET 9.0 1000 G 106.90 μs 0.240 μs 0.201 μs 106.90 μs 1.05 0.00 5.4932 93088 B 1.06
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 G 391.88 μs 2.477 μs 2.196 μs 391.12 μs 1.00 0.01 19.0430 121461 B 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 G 388.93 μs 1.169 μs 0.976 μs 388.80 μs 0.99 0.01 20.5078 129901 B 1.07
MassToString .NET 9.0 .NET 9.0 1000 N 117.68 μs 2.083 μs 3.480 μs 116.08 μs 1.00 0.04 4.8828 82872 B 1.00
VolumeFlowToString .NET 9.0 .NET 9.0 1000 N 117.19 μs 0.224 μs 0.187 μs 117.21 μs 1.00 0.03 5.3711 90912 B 1.10
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 N 395.04 μs 2.405 μs 2.132 μs 394.57 μs 1.00 0.01 19.0430 121117 B 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 N 385.76 μs 0.515 μs 0.430 μs 385.65 μs 0.98 0.01 20.0195 126781 B 1.05
MassToString .NET 9.0 .NET 9.0 1000 S 151.99 μs 0.379 μs 0.336 μs 152.03 μs 1.00 0.00 11.2305 191912 B 1.00
VolumeFlowToString .NET 9.0 .NET 9.0 1000 S 156.02 μs 0.202 μs 0.189 μs 156.01 μs 1.03 0.00 11.7188 197088 B 1.03
MassToString .NET Framework 4.8 .NET Framework 4.8 1000 S 650.07 μs 3.216 μs 3.008 μs 649.23 μs 1.00 0.01 89.8438 570782 B 1.00
VolumeFlowToString .NET Framework 4.8 .NET Framework 4.8 1000 S 675.46 μs 3.121 μs 2.606 μs 674.95 μs 1.04 0.01 91.7969 579223 B 1.01

PS Shit, forgot to switch the framework target for the before (it's NET8).. anyway, you see the lack of allocations on the A format.
PS2 I've updated the results, and interestingly NET9 (before) performed slightly worse on the A format..

@lipchev lipchev requested a review from angularsen April 19, 2025 16:14
@lipchev
Copy link
Collaborator Author

lipchev commented Apr 19, 2025

@angularsen Unless you have something to add, I think this is ready to go. Tell me how you want to deal with the ToString stuff, and I'll open another PR.

PS I need to get into the shower or I'm going to be late, so when you're ready just merge it. I'll be back later tonight..

@angularsen
Copy link
Owner

Yeah let's merge this and address topics in separate PRs.

  1. Ok, let's try UnitKey in IQuantity and keeping generics in QuantityFormatter. It took me some time to grok why we couldn't keep UnitKey an internal implementation detail, but I realize now the concrete type must provide the int and Type for the enum to avoid boxing of Enum. The benchmarks show an infinite improvement on allocation, so that is nice 👏

  2. ToString() overloads - let's discuss this separately, preferably with a PR on how you think it should work and with some bullet points on pros/cons of ways to do it. It will be easier for me to comment.

@angularsen angularsen merged commit bd4282f into angularsen:master Apr 19, 2025
1 check passed
@angularsen angularsen added this to the v6 milestone Apr 19, 2025
@angularsen angularsen changed the title QuantityFormatter: introducing an UnitAbbreviationsCache instance and Format-ing using a generic TQuantity QuantityFormatter: Take UnitAbbreviationsCache instance, Format with TQuantity Apr 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add the possibility to instantiate a QuantityFormatter
2 participants