Add String.Split overloads that take a single char and string separator

[justinvp]

It's extremely common to split a string based on a single char or string separator, yet String.Split only offers overloads that accept an array of separators. The params char[] separator overload is particularly insidious as the params usage often results in a large number of unnecessary char[] heap allocations, unbeknownst to the developer.

Rationale and Usage

Stack Exchange ran into this (significant number of separator arrays in memory) and submitted a pull request to ASP.NET MVC to cache String.Split separator arrays inside the ASP.NET MVC codebase.

They ended up with the following:

namespace System.Web.Mvc
{
    internal static class StringSplits
    {
        // note array contents not technically read-only; just... don't edit them!
        internal static readonly char[]
            Period = new[] { '.' },
            Comma = new[] { ',' };
    }
}

With uses of string.Split('.') and string.Split(',') replaced with string.Split(StringSplits.Period) and string.Split(StringSplits.Comma) to avoid the char[] allocations.

It'd be awesome if String.Split offered overloads that accepted a single separator to avoid this.

Proposed API

public partial class String
{
    // Proposed methods
    public string[] Split(char separator, StringSplitOptions options = StringSplitOptions.None);
    public string[] Split(char separator, int count, StringSplitOptions options = StringSplitOptions.None);
    public string[] Split(string separator, StringSplitOptions options = StringSplitOptions.None);
    public string[] Split(string separator, int count, StringSplitOptions options = StringSplitOptions.None);

    // Existing methods
    public string[] Split(params char[] separator);
    public string[] Split(char[] separator, int count);
    public string[] Split(char[] separator, StringSplitOptions options);
    public string[] Split(char[] separator, int count, StringSplitOptions options);
    public string[] Split(string[] separator, StringSplitOptions options);
    public string[] Split(string[] separator, int count, StringSplitOptions options);
}

Notes

We had wanted to add public string[] Split(string separator), but this breaks source compatibility with uses of Split(null), which is documented to split based on white space, because it makes the call ambiguous between Split(char[]) and Split(string).

I won't go into it here as it's really a separate feature request, but it would also be worth considering new Split methods that return a struct collection of StringSpan/StringSegment structs (in lieu of internal runtime span/substring magic) to avoid the resulting string[] allocation (and substring allocations, unless needed).

I'd be happy to contribute an implementation and tests.

[ellismg]

I won't go into it here as it's really a separate feature request, but it would also be worth considering new Split methods that return a struct collection of StringSpan/StringSegment structs (in lieu of internal runtime span/substring magic) to avoid the resulting string[] allocation (and substring allocations, unless needed).

Please open an issue on this. We have been talking about it from time to time internally (and I've done a fair amount of thinking and prototyping) but we should open the discussion up. There are lots of interesting issues to consider.

[ellismg]

I think this would be a great addition to the framework.

[svick]

Depending on if and how slices are implemented, that StringSpan version might not be necessary, I think.

[weshaggard]

@justinvp I think these are some reasonable asks and I think the discussion should happen here in the corefx repo regardless of whether we end up putting these directly on string in the coreclr repo or not.

I also like the other idea of a generally more efficient string split that uses structs and agree that it should be a separate issue but it is worth mentioning here and considering it along with these to have a more holistic look at our different split options.

One question I wonder about this is do you have any numbers that show how much of an improvement something like this might have? Knowing that split is really bad about allocations is the actual params array really going to show as much of an improvement in the overall amount of allocations that occur or will it just be a tiny improvement and switching to the struct split is ultimately what people should do to get the performance they need. In other words is the params array 1 of 20 allocations or 1 of 2 allocations in this code. I suspect it depends on the number of items that end up in the split string array but it is worth thinking about.

[mattwarren]

+1 👍 from me.

I like the general trend I see in a lot of the new work in .NET. There seems to be a concerted effort to ensure there are as few allocations as possible (i.e. stuff like this https://github.com/dotnet/corefxlab/wiki)

[pharring]

string.Split is an awful allocation hog and I've learned to avoid it. Not only the params char[] args and the obvious returned array, but the implementation allocates an int[] for all the split locations.
Often, string.Split is unnecessary. Iteration with string.IndexOf is allocation free and, if you really need substrings, well those allocations are unavoidable.

I think an alternative API which returns a struct-based enumerator of spans would be ideal - similar to the note in the proposal. It would encourage people to think more of "parsing" and less about "splitting".

[krwq]

While I agree on a single character overload, the string overload is not obvious to me. I.e.: What is the expected result of "aaabaaabaaa".Split("aa")? ["", "ab", "ab", "a"] or rather ["a", "b", "ab", "a"]? Or should this string be treated as char[] overload and work the same as that?

[mgravell]

What @pharring says here makes a lot of sense - especially with the immutability of strings making it trivially repeatable; a custom struct iterator would be virtually free (until, of course, someone uses LINQ on it, causing it to box - but hey; we can't have everything).

[pharring]

@krwq the matching rules for the string overload would be the same as they are today with the char[] version. The string is just an alternative way of packaging the set of separator chars. Repeated chars are ignored.

[krwq]

@pharring, in this case I don't think this API is a good idea. In my opinion it is gonna be confusing with splitting a string by separator which is string. I'd rather have slow API which intentions are clear. Can we expose some non allocating API which gives you ImmutableArray from string which will have a char[] underneath which we can consume without new allocations? Besides we usually call Split quite rarely so what's the point on saving few nanoseconds/bytes per instance?

[davkean]

@krwq I don't understand your concern, this proposal does not add any new features to String.Split. As @pharring mentioned, this is just a way to avoid the initial allocation of passing the separators to String.Split.

Introducing a new non-allocating String.Split is a great idea that we've talked about a lot in the past, however, it should be treated separably to this proposal and not as an "either or".

[ellismg]

@pharring @krwq I would assume the string versions would just be aliases for the string[] versions. In this case:

"aaabaaabaaa".Split("aa")

behaves the same way as

"aaabaaabaaa".Split(new string[] { "aa" }, StringSplitOptions.None)

So you would get:

"", "ab" , "ab", "a"

[eatdrinksleepcode]

I think what @krwq is saying is that if the method call is "Split(String separator)", it is not clear whether the separator is a single multi-character separator or a collection of single-character separators expressed as a string.

I agree that this would be confusing and would prefer that it not be included. Despite the fact that String implements IEnumerable, I can't think of another API that treats a string as a collection of characters in this way.

On Apr 28, 2015, at 11:25 AM, David Kean notifications@github.com wrote:

@krwq I don't understand your concern, this proposal does not add any new features to String.Split. As @pharring mentioned, this is just a way to avoid the initial allocation of passing the separators to String.Split.

Introducing a new non-allocating String.Split is a great idea that we've talked about a lot in the past, however, it should be treated separably to this proposal and not as an "either or".

—
Reply to this email directly or view it on GitHub.

[ellismg]

I think what @krwq is saying is that if the method call is "Split(String separator)", it is not clear whether the separator is a single multi-character separator or a collection of single-character separators expressed as a string.

I would have expected that most folks would assume that

"this, is, a, string, with some spaces".Split(", ");

is going to give you back

"this", "is", "a", "string", "with some spaces"

and not

"this", "", "is", "", "a", "", "string", "", "with", "some", "spaces"

I could maybe buy the argument if we didn't have any existing overloads of Split that took strings, but we already have clear semantics for how to split using one or more strings as a splitter.

[eatdrinksleepcode]

@ellismg I agree with your interpretation, but @pharring's explanation seems to indicate otherwise.