Add new overloads with ReadOnlySpan to System.Net.NetworkInformation.PhysicalAddress

[iSazonov]

Rationale

Coming from #26188.

Let's to consider a scenario of DHCP log parsing. We read line by line and parse the line to get fields. Previously we used Substring() method. My understanding is that now best practice (from Stephen Toub) is to use Slice() to exclude extra allocations. So suggested overloads with ReadOnlySpan can be useful.

It is very perf-sensitive - sizes of DHCP logs can reach huge sizes in enterprises. We could use perl or php (or sed/awk) but why we should refuse favourite C# and PowerShell?

About "The API is rare". (1) Although it is rare but we all already have this Parse (string address); - why if it is rare? (2) Why such APIs should remain not optimized? Adding and having ReadOnlySpan overloads is too expensive? I'd like to see a policy and guidance from your team.

Proposed API

namespace System.Net.NetworkInformation
{
    public class PhysicalAddress
    {
        public static bool TryParse(string address, out PhysicalAddress value);
        public static bool TryParse(ReadOnlySpan<char> address, out PhysicalAddress value);
        public static PhysicalAddress Parse(ReadOnlySpan<char> address);
        // Existing
        // public static PhysicalAddress Parse(string address);
    }
}

[ghost]

Would it make sense to move public static bool TryParse(string address, out PhysicalAddress mac); to this proposal as well and leave #26188 scoped to improve the parsing logic?

[karelz]

IMO we should not merge TryParse into this API request. TryParse is about avoiding exceptions in non-exceptional cases. Span is about critical perf.

[karelz]

(1) Although it is rare but we all already have this Parse (string address); - why if it is rare?

We have Parse API, because it is sometimes (albeit rarely) useful. Having the capability is valuable (as its usage shows), but it in general does not justify to have the capability hyper-optimized, especially when the optimization is not free (adding new API).

(2) Why such APIs should remain not optimized? Adding and having ReadOnlySpan overloads is too expensive? I'd like to see a policy and guidance from your team.

Adding API is not free. It has cost also on platform usability (choice from more overloads adds tax on developers).
AFAIK so far we added Span overloads only to types where we saw clear perf benefits in perf-sensitive scenarios (e.g. web servers). We held back on the rest. We may reconsider that in future and adjust our position.

Personally, I am skeptical that the Span overloads would help to make any real-world DHCP parser app measurably faster. It feels more like premature optimization to me, but maybe I am wrong about it or there are other scenarios where it matters.

On related note: If there was no Parse API at all and we would be adding it from scratch - would we choose string or ReadOnlySpan<char>? ... I am not sure even about that, given that it is not high-performance parser. For me the simplicity and familiarity of string would probably win over potential perf advantage.

[ghost]

IMO we should not merge TryParse into this API request. TryParse is about avoiding exceptions in non-exceptional cases. Span is about critical perf.

Then TryParse maybe doesn't belong to https://github.com/dotnet/corefx/issues/29740 either, rather than in its own proposal. This proposal is about Parse and TryParse overloads, where the latter one doesn't exist. So that should be reworded as well, right?

[iSazonov]

@karelz Thanks for sharing .Net Core team policy!

Adding API is not free.

I agree that the choice of a large number of APIs adds complexity for developers. But really this complication appears if similar things are done in different places (namespaces).
My point is that Spans is a special case - we do not add new APIs, we make a mirror of the existing ones. I would expect that such Span overloads exist for most existing string APIs.
The point comes from PowerShell Core engine code analyses. PowerShell likes to convert everything into strings. So the more APIs have Spans the faster the engine we can make. (This is certainly a generalization).
PowerShell can't use Spans in scripts but PowerShell can do a lot of magical things. For example, it allows you to delegate script blocks. If we learn how to use Spans there, our users will be able to create much more efficient scripts, but only if APIs similar to those discussed in this topic will have Spans overloads.

[iSazonov]

@kasper3

This proposal is about Parse and TryParse overloads , where the latter one doesn't exist

I believe the topic is a successor of #26188 - if #26188 will be rejected the suggestion will be rejected too. In any case I agree to reword. Feel free to edit the issue description.

[karelz]

Triage: Networking team is fine to take these APIs for larger review.

[terrajobst]

Video

We should also have string-based TryParse method. We should also name the out parameter value. Other than that, looks good as proposed.

namespace System.Net.NetworkInformation
{
    public class PhysicalAddress
    {
        public static bool TryParse(string address, out PhysicalAddress value);
        public static bool TryParse(ReadOnlySpan<char> address, out PhysicalAddress value);
        public static PhysicalAddress Parse(ReadOnlySpan<char> address);
        // Existing
        // public static PhysicalAddress Parse(string address);
    }
}

[MihaZupan]

@terrajobst What is the use-case for a string-based TryParse if we're adding a ROS<cbar> overload?