@AlexGhiondea Does this whole thing do anything useful for .NET Core?

Isn't this effectively called only once per application run? I'd opt for code readability (current, pre-PR) over the trivial amount of allocations for this scenario.

@NickCraver Looks like you're right about it being run once per app. I had hoped that refactoring it into different methods would at least make up for some of the increased complexity, though.

Perhaps I should only refactor the Split allocation that happens per-component (which is the one I pointed out in my description); that one is much easier to refactor and shouldn't require major refactoring.

@jamesqo IMO it's still so low on the allocations - go for simpler code. That being said, I would refactor this:

const char c_componentSeparator = ',';
const char c_keyValueSeparator = '=';

There's no need to create the same parameter arrays every time e.g.:

static readonly char[] c_componentSeparator = { ',' };
static readonly char[] c_keyValueSeparator = { '=' };

The code for this was originally ported and made to now throw from the code that parses FrameworkName

So maybe it is useful to make a similar change there.

@jkotas this is used by CoreCLR in a similar way we use it for Desktop. But I don't think we have any quirks that are enabled based on this machanism in K.

This code is only going to run once per add-domain (which is once per app for CoreCLR) which would slightly reduce the impact of this change.

I will take a closer look at this Monday but I agree with @NickCraver that code readability should be something to consider when the perf improvement is not significant.

Overall the code changes are fine. However, I am not convinced that we are saving this much by doing this. Do you have some perf numbers to show the difference?

Another thing to consider is turning this into a 'mini-parser' that will traverse the string only once and just allocate the substrings it needs to return (like the identifier and profile). While the code would certainly be more complex, it would reduce the number of allocations to a minimum.

I'm not so sure this pull request is worth it anymore... I'm going to close this for now.