DOTNET_PROCESSOR_COUNT is a hex value -- should be decimal

[richlander]

DOTNET_PROCESSOR_COUNT is treated as a hex value. That's unexpected and likely to surprise users. It should be changed to a decimal value.

Context: #53149

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[janvorli]

@richlander we need to be careful here. All config values in env variables that .NET supports are hexadecimal and and it was this way forever. So changing it to decimal would be a breaking change for many people. Unless we start treating the env vars prefixed by DOTNET_ differently from the COMPlus_ prefixed ones.

[richlander]

All config values in env variables that .NET supports are hexadecimal

I didn't know that. If that's the case, that's fine. I was looking at the behavior from the myopic viewpoint of this scenario. I'm OK just documenting this behavior.

I'll close the issue. If others have an alternate view, feel free to offer that.

[jkotas]

All config values in env variables that .NET supports are hexadecimal

That was true for the existing COMPlus_XXX env variables. It is not true for the existing DOTNET_XXX env variables. All(?) existing DOTNET_XXX env variables treat numbers as decimal by default. Just one example from many: DOTNET_SYSTEM_NET_SOCKETS_THREAD_COUNT is decimal.

So we are going to have inconsistency here in any case. I think we should use the more sensible convention (ie decimal by default) for anything new, especially if it is going to be used frequently like DOTNET_PROCESSOR_COUNT

[AntonLapounov]

Almost all numeric COMPlus_/DOTNET_ environment variables are treated as hexadecimal, because CLRConfig::GetConfigValue has always worked that way:

runtime/src/coreclr/pal/src/init/pal.cpp

Lines 413 to 415 in 01b7e73

	// Like all numeric values specific by the COMPlus_xxx variables, it is a
	// hexadecimal string without any prefix.
	long int flag = strtol(useDefaultBaseAddr, NULL, 16);

runtime/src/coreclr/utilcode/clrconfig.cpp

Line 241 in 01b7e73

DWORD configMaybe = wcstoul(val, &endPtr, 16); // treat it has hex

We have this behavior documented for GC configuration settings (COMPlus_GCHeapCount, COMPlus_GCHeapHardLimit,
COMPlus_GCHeapHardLimitPercent, COMPlus_GCHighMemPercent, etc.):

For number values, use decimal notation for settings in the runtimeconfig.json file and hexadecimal notation for environment variable settings. For hexadecimal values, you can specify them with or without the "0x" prefix.

For a couple of other numeric options we have added custom processing to treat them as decimal.

I do not have a strong opinion here, but I do not see why it could be unexpected for the PROCESSOR_COUNT to be treated as hexadecimal given that GCHeapCount and other options have been treated as hexadecimal for a number of .NET releases.

especially if it is going to be used frequently like DOTNET_PROCESSOR_COUNT

I consider using this setting rather an advance scenario.

[AntonLapounov]

It is not true for the existing DOTNET_XXX env variables. All(?) existing DOTNET_XXX env variables treat numbers as decimal by default.

I think this is a bit confusing statement given that for most configuration settings DOTNET_XXX is a synonym for COMPlus_XXX and they are treated exactly the same way and, as far as I remember, the long-term plan was to obsolete COMPlus_XXX variants.

Also I may be missing something but I found only two numeric DOTNET_XXX environment variables treated as decimal:

• DOTNET_SYSTEM_THREADING_POOLINGASYNCVALUETASKSCACHESIZE
• DOTNET_SYSTEM_NET_SOCKETS_THREAD_COUNT

While this DOTNET_XXX environment variable is treated as hexadecimal:

• DOTNET_DefaultDiagnosticPortSuspend
  

  runtime/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h

  Line 182 in d156b9e

  return static_cast<uint32_t>(CLRConfig::GetConfigValue (CLRConfig::EXTERNAL_DOTNET_DefaultDiagnosticPortSuspend));

Update: It seems that DOTNET_DefaultDiagnosticPortSuspend is rather a Boolean flag as we do not use its numeric value, but only check whether it is greater than zero.

[richlander]

Here's a good principle:

• The ENV and the the FX property it affects should be oriented on the same numeric type.

One could imagine building a Monte Carlo machine to validate best performance. If you log Environment.ProcessorCount, then that value and the value you used for the ENV won't match. That's super unfortunate. I'm essentially articulating round-tripping of data.

I don't think it's super useful to focus on how the system work, but focus on what we think the best UX is. You've particularly that there is a mix today of approaches. Which approach do we like best?

[janvorli]

Decimals would obviously be more user friendly for stuff like counts and less user friendly for bitmasks. We use the env vars for both kinds of values. I am starting to think that it would make sense to handle the DOTNET_ prefixed env vars differently from the COMPlus_ prefixed ones w.r.t. the numeric base. For the new DOTNET_ prefixed ones, we could use decimal as the default, but allow passing in radix prefix like 0x or 0b to specify other base. I think that the COMPlus ones should keep working as before, otherwise we would break people.

[jkotas]

I am starting to think that it would make sense to handle the DOTNET_ prefixed env vars differently from the COMPlus_ prefixed ones w.r.t. the numeric base.

+1

We have done the grand unification of COMPlus_ and DOTNET_ just a few weeks ago, and we are starting to find places where it does not hold well together.

@AaronRobinsonMSFT Thoughts?

[AaronRobinsonMSFT]

I think that the COMPlus ones should keep working as before, otherwise we would break people.

Agree.

but allow passing in radix prefix like 0x or 0b to specify other base.

Processing the radix is possible, although I am unsure how valuable in practice.

I am starting to think that it would make sense to handle the DOTNET_ prefixed env vars differently from the COMPlus_ prefixed ones w.r.t. the numeric base.

I am leery of having the prefix dictate the semantic here. It wouldn't be hard to add a flag that says "interpret as dec" and default to hex. This would take me about 30 minutes to add. Basically have the definer of the config setting state what base they want to use. I would argue the cases where both radix types should be supported for a config are close to 0.

[jkotas]

It wouldn't be hard to add a flag that says "interpret as dec" and default to hex.

That only works if you have one and only party setting the configs. If you have two different parties setting the different configs, how are they going to agree on what to use?

[AaronRobinsonMSFT]

If you have two different parties setting the different configs, how are they going to agree on what to use?

Are you referring to the case where we have a groups of people that have a preference between hex or dec? I don't see that as a reality. Each config has a historical format - prior to the unification of DOTNET_ and COMPlus_. That historical format is the default (i.e., hex) or they can define dec as the new preference. Can you provide a concrete example where this would be an issue? There is also the use case that permits reading the string and parsing as desired. Also, respecting a radix prefix is viable too.

[AntonLapounov]

Here's a good principle:

• The ENV and the the FX property it affects should be oriented on the same numeric type.

The numeric type here is int consisting of 32 binary digits, which is arguably closer to the hexadecimal representation than to the decimal one. :)

If you log Environment.ProcessorCount, then that value and the value you used for the ENV won't match.

They would match if you log it using the same base, i.e. as a hexadecimal number.

I don't think it's super useful to focus on how the system work, but focus on what we think the best UX is. You've particularly that there is a mix today of approaches. Which approach do we like best?

If I designed from scratch, I would likely parse numbers as decimal by default and allow the 0x prefix for hexadecimal values. I am not sure it would be worth to support binary (0b) or octal (0) numbers. In the present situation, it is pretty unfortunate to have to use different bases for related settings like PROCESSOR_COUNT and GCHeapCount.

[richlander]

The numeric type here is int consisting of 32 binary digits, which is arguably closer to the hexadecimal representation than to the decimal one. :)

There are two meanings of decimal here and I mean only one of them. I mean "base 10 ints". As you well know, both base 10 and base 16 numbers are fully representable as base 10 integer.

Your answer appears to be missing that point.

[AntonLapounov]

Per-setting flag would be a cleaner alternative to existing custom processing as decimal here and there.

If you have two different parties setting the different configs, how are they going to agree on what to use?

You might be thinking about a separate flag affecting parsing all settings, but that is not what was suggested here.

[AntonLapounov]

There are two meanings of decimal here and I mean only one of them. I mean "base 10 ints".

There is a dedicated System.Decimal type for base-10 numbers. The Environment.ProcessorCount property is typed as int, not as decimal or hexadecimal, so I fail to understand this argument.

[richlander]

My primary point was that the numeric form of the ENV and the property should match. They don't currently. One is hex and the other is decimal, as I called out.