More consistent formatting of results

[jodydonetti]

Would it be possible to have a more consistent formatting of results, to avoid situations like the one below where numbers are not aligned, making it somewhat harder to scan trough results?

I'm confident I'm probably missing some important detail here 😅 .

ps: thanks for BDN, it truly is a work of beauty.

[adamsitnik]

Hi @njy !

Would you be interested in sending a PR?

It should be a matter of changing the format here:

https://github.com/dotnet/BenchmarkDotNet/blob/be769f8e5012cdd818f2af7c6c9d9f18872c054a/src/BenchmarkDotNet/Diagnosers/MemoryDiagnoser.cs#L49

[jodydonetti]

Hi @njy !

Would you be interested in sending a PR?

For sure!

It should be a matter of changing the format here:

https://github.com/dotnet/BenchmarkDotNet/blob/be769f8e5012cdd818f2af7c6c9d9f18872c054a/src/BenchmarkDotNet/Diagnosers/MemoryDiagnoser.cs#L49

Ok I'll clone the repo and try to do that, thanks for the guidance on where to look at.

[jodydonetti]

Probably it's not that, at least not only that, because as you can see the other rows (of the same column) are formatted differently (definitely not N0 since there are 2 positions after the decimal separator). I thought about an automatic change for the NumberFormat when the Unit change, but all of them are in MB. I thought it may have been a different related to rounding, but the first row is near the next round value while the others are near the lowest one.

Keep investigating 🔍

[jodydonetti]

Trying to reverse engineer the flow I'm betting about this about this:

https://github.com/dotnet/BenchmarkDotNet/blob/master/src/BenchmarkDotNet/Columns/MetricColumn.cs#L36

[jodydonetti]

So here are my findings.

It seems inconsistent that at this line

https://github.com/dotnet/BenchmarkDotNet/blob/master/src/BenchmarkDotNet/Columns/MetricColumn.cs#L36

the format is not passed, whereas it is passed at this line

https://github.com/dotnet/BenchmarkDotNet/blob/master/src/BenchmarkDotNet/Columns/MetricColumn.cs#L40

So my idea at first would've been to change this

return SizeValue.FromBytes((long)metric.Value).ToString(style.SizeUnit, cultureInfo);

to this:

return SizeValue.FromBytes((long)metric.Value).ToString(style.SizeUnit, cultureInfo, descriptor.NumberFormat);

But that, in theory, would make the number format fixed.

Btw in the method that gets called, this

https://github.com/dotnet/BenchmarkDotNet/blob/be769f8e5012cdd818f2af7c6c9d9f18872c054a/src/BenchmarkDotNet/Columns/SizeValue.cs#L41

it seems as if the default format is 0.## and that, in the end, is probably the main thing to think about.

The thing is that the format 0.## is as you know adaptive, that is it can decide how many digits to show based on the specific value (up to the number of # characters) so 123.321 would be outputted as "123.32" whereas 123.001 would be outputted as simply "123".

This in turn may generally look like a good idea, but considering that every row will be formatted independently, that will cause the inconsistency problem I highlighted initially. So the idea may be to just change every occurrence of 0.## with the non-adaptive version 0.00.

But searching for the 0.## pattern gives me back 14 occurrences, so if I make such a change I suppose it may have unintended consequences - at least in theory - and I would like to know if you think this would be the right direction to go towards.

Please let me know what you think, since you obviously know the codebase way better anybody else.

Thanks!

[adamsitnik]

But that, in theory, would make the number format fixed.

this idea sounds very good to me! Could you please give it a try?

So the idea may be to just change every occurrence of 0.## with the non-adaptive version 0.00.

@njy it sounds good to me, but I need a confirmation from @AndreyAkinshin

@AndreyAkinshin is there any good reason for using 0.## instead of 0.00 as the default format?

[jodydonetti]

Great @adamsitnik , I'll wait for @AndreyAkinshin 's confirmation, then will proceed with the PR.

[adamsitnik]

I'll wait for @AndreyAkinshin 's confirmation, then will proceed with the PR

@nju Andrey is very busy recently, you can send the first PR with a fix for memory, and after Andrey responds you can send 2nd PR that would change the default formatting

- return SizeValue.FromBytes((long)metric.Value).ToString(style.SizeUnit, cultureInfo);
+ return SizeValue.FromBytes((long)metric.Value).ToString(style.SizeUnit, cultureInfo, descriptor.NumberFormat);

[jodydonetti]

Hi @adamsitnik , PR #1569 is ready.

ps: you reference another user ("nju") in your previous reply, I am "njy" 😉

[adamsitnik]

@njy big thanks for the PR and apologies for the typo!

[jodydonetti]

Great, thanks to you for the support.

One last thing: regarding the change (14 different occurrences) from 0.## to 0.00 would you prefer to wait for Andrey to say something or should I create another PR for that and discuss the change there?

I'm asking to avoid creating extra PR for you to track.