Runtime: String enhancements

[juliusfriedman]

Background and Motivation

Because we force people to ask for the length we already know

Proposed API

public class String{
   //Indicates if the string is null or empty (length == 0), outputs the length to len in the success case.
+ public static bool IsNullOrEmpty(String str, out int len);
+ public static bool IsNullOrEmptyOrWhitespace(String str, out int len);
}

Usage Examples

if(!String.IsNullOrEmptyOrWhitespace(someString, out int len) && len > 10){
//
}else{
//
}

Alternative Designs

Extension methods

cc @scalablecory

[Dotnet-GitSync-Bot]

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.

[danmosemsft]

@juliusfriedman sorry, this is not enough justification.

As I mentioned in another issue, it is not enough for an API to be potentially useful to some people. New API also comes with cost: maintenance and cognitive burden for potentially 10, 20 or more years. Consider how many times people will hit "." in Visual Studio on String. This is a new entry in the list they see. It has to earn its place. We know from API usability studies that 'out' parameters are not as useable, and must earn their place - usually by being significantly faster in performance sensitive codepaths.

What code pattern would this improve? Would it help perf significantly, meaingfully reduce confusion or just save a line of code? Can you point to 5-10 existing instances of this pattern in a variety public codebases, that would be improved by adding this API?

[danmosemsft]

Another way of thinking about it -- consider String to be similar to std::string in the C++ standard library -- as you imagine, they have their own process and threshold for adding new members to it. Perhaps ours is not quite as high but it's not an unreasonable analogy. I don't want to deter you from making proposals - just trying to help suggest how they can be most effective.

[juliusfriedman]

But these are used on hot paths and would reduce the number of calls therein by 2, not everyone caches length, perhaps there should be analyzer for that. You also make the opposite argument in https://github.com/dotnet/runtime/issues/39565 to put them on the class itself, these could be extensions also and intrinsic at the same time.

[danmosemsft]

It would be good to have evidence that in real scenarios, this difference is measurable. (Eg., using Benchmark.NET)

It looks like the method is already inlined (see sharplab).

Essentially the diff is a redundant 'je' and 'cmp', and also unnecessarily pushing ebp (this may be lost in noise in a real method doing other work). If this is measurable in real scenarios, it might be more interesting to see whether the codegen can be improved, rather than creating a new API for all developers to change to.

[juliusfriedman]

It would be good to have evidence that in real scenarios, this difference is measurable. (Eg., using Benchmark.NET)

It looks like the method is already inlined (see sharplab).

Essentially the diff is a redundant 'je' and 'cmp', and also unnecessarily pushing ebp (this may be lost in noise in a real method doing other work). If this is measurable in real scenarios, it might be more interesting to see whether the codegen can be improved, rather than creating a new API for all developers to change to.

What about handling many different lengths in a switch expression?

e.g.

 str switch{ s when str.Length > 4 && str.Length < 8 }

i think

But still, the point it what if I just want to display the length?

The big point is that the underlying methods check the length and dont give it back, e.g. IsNullOrEmpty etc

[danmosemsft]

Again, where is the demonstrated performance impact? I mean, measurable, significant, in realistic code.

[juliusfriedman]

Look at the code yourself it's huge...

String v = "akjadsjg'lkagsj'";
        if(string.IsNullOrEmpty(v)) return;
   //     int z = v.Length;
        for(int i = 0; i < v.Length; ++i)
        {

        }

[danmosemsft]

I am asking for measurements that show significant performance impact in realistic code. Eg., using Benchmark.NET.

[juliusfriedman]

Can you provide an example template please

[GrabYourPitchforks]

Hi Julius! Thanks for the suggestion. I think part of the confusion stems from that the team's not quite sure what scenario this would help. Consider the two examples below.

string s = ...;

// Pattern 1
if (!string.IsNullOrEmpty(s) && s.Length > 10)
{
    // use s
}

// Pattern 2
if (!string.IsNullOrEmpty(s, out var length) && length > 10)
{
    // use s
}

These two patterns would result in the exact same x86 codegen. Furthermore, Pattern 1 is more "natural" to the typical C# developer. So who is the target audience for Pattern 2, and what scenario would it help?

[juliusfriedman]

2 would be for lexers or parser but if the optimization is done at the JIT then the method is useless, It is good to know this is already the case in .net core.

Can you show the case where the output asm is the same? It might be late but I am having a hard time getting to see that same result in something like sharp labs.

Please also see: https://github.com/dotnet/runtime/issues/39562

[GrabYourPitchforks]

As the claimant, the burden of proof that this new API helps perf lies with you. :)

But to your question about the output asm, we can do it through a thought exercise. Say that the local _s_ is already in a register rdx. To check for null, it's a test against zero.

test rdx, rdx
jz <target_was_null>

Now here's the check against zero ("is empty?"):

cmp 0, dword ptr [rdx + offset_to_length]
jbe <target_was_empty_string>

These first two checks are part of IsNullOrEmpty. Now here's the check against 10 in the caller's code:

cmp 10, dword ptr [rdx + offset_to_length]
jg <target_was_less_than_10>

All three of those checks must take place, regardless of which pattern you used. (Pedantic point: the second and third checks could be merged together, but that's a JIT optimization. And that optimization would be independent of which pattern the caller used.)

[juliusfriedman]

https://sharplab.io/#v2:EYLgtghgzgLgpgJwDQxNMAfAAgJgIwCwAUFgMwAEu5AwuQN7HlOUVYAs5AsngBQCU9Rs2EBlGAgCWAOwDm5AG7kAvOQBEEANYArCABMoWmQHIANhogyDR1QG4hwphIBmPLHgAMAOgCSUAHIAriYmAPIIAKJgAA4wAJ488nwCWADsdkTM9g5OAPYIPNIw5BLK5O42xeQAPAqeADJwsjAAFhUA1G0SfFnCDBkOAz3MAL5Zo/1MWVlklBycOPyCEw5ikrIKperaegbGZhZWtkOOLm5evoHBYZEx8YnJacfFUkUAXqXy9Y0yLekDTLl8oVKipypUaq92p1ustek9hE9xsIkZkJsRhkA=

I don't follow your example... perhaps It's late I will read it again tomorrow.

The call should put the length in a register so as you say so that both parts can be combined. I agree about the JIT optimization part, I understand the JIT could and should recognize these patterns.

I think this is even more useful on Array.

About to hit the sack, thanks for your time and ttyt

[danmosemsft]

I’m going to close this for the reasons stated above.

[GrabYourPitchforks]

The JIT has some extra logic to enregister the string's length when it sees that the length is dereferenced as part of the loop condition. In your sample you're seeing an interaction of string.IsNullOrEmpty and this JIT optimization. There are some edge cases where the JIT could be smarter (such as skipping the "is empty?" comparison if the caller's going to turn around and compare against 10 anyway), but again, that's wholly a JIT issue and is unrelated to the API proposed here.

[juliusfriedman]

The JIT has some extra logic to enregister the string's length when it sees that the length is dereferenced as part of the loop condition. In your sample you're seeing an interaction of string.IsNullOrEmpty and this JIT optimization. There are some edge cases where the JIT could be smarter (such as skipping the "is empty?" comparison if the caller's going to turn around and compare against 10 anyway), but again, that's wholly a JIT issue and is unrelated to the API proposed here.

Even though the API provides a way around ur JIT that is straight forward?

And it's a common thing to do on arrays.. thankz for ur time. & for the record I am very disappointed.

I didn;t even have to prove my case for MatchTimeout nor tail calls as hard as I had to do for something so trivial as this.

[GrabYourPitchforks]

Even though the API provides a way around ur JIT that is straight forward?

A reminder: you still haven't demonstrated that. :)

What you did demonstrate is that the JIT produces different codegen when the Length property is dereferenced before the loop begins vs. within the loop itself. And this is already known. The linked example doesn't use string.IsNullOrEmpty and shows the difference very clearly.

for the record I am very disappointed

Why? I think at its heart your issue is "I wrote some code and expected _abc_ codegen but instead got _xyz_ codegen." And those issues are immensely valuable. The JIT team is already tracking many such issues, and the community is definitely encouraged to open more issues as needed. But we don't tend to introduce APIs _solely_ for things that should be solved by the JIT. Instead we prefer to make changes to the JIT itself, introducing new APIs only if those APIs have utility separate from any JIT optimizations.

[juliusfriedman]

Even though the API provides a way around ur JIT that is straight forward?

A reminder: you still haven't demonstrated that. :)

What you did demonstrate is that the JIT produces different codegen when the Length property is dereferenced before the loop begins vs. within the loop itself. And this is already known. The linked example doesn't use string.IsNullOrEmpty and shows the difference very clearly.

for the record I am very disappointed

Why? I think at its heart your issue is "I wrote some code and expected _abc_ codegen but instead got _xyz_ codegen." And those issues are immensely valuable. The JIT team is already tracking many such issues, and the community is definitely encouraged to open more issues as needed. But we don't tend to introduce APIs _solely_ for things that should be solved by the JIT. Instead we prefer to make changes to the JIT itself, introducing new APIs only if those APIs have utility separate from any JIT optimizations.

How can there be any argument that needs proof that the method would not give you an out that you can totally eliminate later, we are already improving multi reg returns... what do you want me to provide to prove it?

it's a bit nebulous and I can't seem to appreciate that.

I can appreciate your time and hard work and that is that.

[GrabYourPitchforks]

what do you want me to provide to prove it?

Here's what I'd recommend to get traction on this. Separate _the problem_ from _your perceived solution_. The issue as currently stated here provides a solution but doesn't clearly state what problem it's trying to solve.

You obviously have a sample in mind, so focus on that. Open a new issue and include three things: (a) a sample C# application that produces inefficient codegen, (b) the actual codegen that is produced, and (c) the codegen you think it should produce. You can use sharplab for (a) and (b), as you had done already with this issue. Providing those three pieces of information will clearly identify the problem you're reporting.

The JIT team for the most part doesn't care about managed APIs, so I'd not mention any new API surface in the new issue. This will allow them to tackle the problem from the JIT side of the house. Then we can weigh the solution they come up with vs. the solution proposed in this issue and consider whichever makes the most sense for the ecosystem.

[juliusfriedman]

I feel like I just did that and this is the definition of insanity, my entire lib is built around these functions... Library writers need the length. It gets cached and thats that. Then once the jit is fixed tis fixed and who cares which way you do whatever anymore but the pattern I present is still the most valid and pure. There is basically no discussion on this and that burden I don't feel is on me anymore sorry.

You know this and that's why you guys built Span as an intrinsic and did not do magic around Array like I had suggested and now that is coming back to bite the bit in both cases.

And don't get me wrong levi, your a smart guy but this is idk... weird for me to see you especially a performance minded individual who has a similar open issue to close this one and not see the bigger picture.