typo "charcaters " in a few places

thanks Dan :-)

This should not be needed - CORECLR is always defined.

I have the #else case to avoid compiling the code for non CORECLR. I may be missing something here.

Do you mean CoreRT? I do not think we will want any of CLRConfig.cs in CoreRT.

Unless this code may one day be shared with CoreRT, it will never get compiled without CORECLR defined. It's defined in System.Private.CoreLib.csproj

It may not actually matter, but did you consider reversing this? i.e. starting it at 0 == not present, and then setting it to 1 only once ICU has been detected and all of the function pointers appropriately initialized?

I'll do that

In what situation would ReleaseHandle be called but find InvariantMode == true?

This is not really needed now. I had the initial implementation which was keeping the sort handle but now I don't initialize it at all so it is not needed now. I'll convert this to assert instead

Presumably these arguments have all already been validated?

yes. but I can add some asserts for it

Nit: can you use named arguments for things like the true here and false below? From context I'm assuming it's about first vs last, but it'd be easier to read if it was clear from the call site.

sure. it is good idea anyway :-)

We already have routines for doing this kind of thing (though I don't know if we have it for this exact thing), used for ASCII-only performance paths. Can we look at reusing where possible? That would help to a) ensure the fallback is using well-tested code paths, b) any optimizations to one would apply to the other, and c) for places where we don't currently have ASCII-only fast paths, we might be able to leverage this work to add some.

Related to that, how are we thinking about the performance of this invariant mode? Presumably one of the scenarios for it is apps that don't want to carry the ICU data around because they care about being lean, in which case throughput may also be of importance?

I have already used the existing current optimized implementation in the functionality like string comparisons. but for IndexOf, looks we don't have a version for that. in short, I used whatever exist and provided the missing. Please point me to any implementation I should used if I missed it.
For the performance I agree in general. for the areas we can optimize it more we can look at it and do whatever needed there. I think the current implementation is reasonable for this check in and we can follow up for the optimization later. is this ok?

@benadams who may know of something.

What does it mean that we use both "Invariant" and "Ordinal" in the name? Do they connote different things?

I'll remove the word Ordinal to reduce the confusion. it was meant in the invariant mode we do ordinal comparison.

I do not understand what is work ordinal trying to say.

This check should be after ValidateArguments

will do that

Same here

will do that

This should be after the argument validation at least; or even better in the common worked method.

will move it after the validation. there is no common working method. we have forked implementation for different OS's

How can we get here for the invariant mode?

Yes you are right. I have converted this check to assert instead.

This should be initialized explicitly during startup, not using implicit constructor. The implicit constructor as written will turn into two checks each time. 1. have we initialized already? 2. What the value is? So these checks are twice as expensive than what they need to be.

Or even better - we may want to cache the flag in a field in the objects with hot paths like TextInfo, and check the instance field there. Instance fields are faster to access than static fields.

Since corelib is crossgen'd, I assume the normal tricks around static readonly bools becoming JIT-time consts wouldn't apply, right?

Right, that's why it would be best to cache the bool in instance fields where possible - instance fields are the fastest to access.

sorry I am not clear about that. I am using this bool static in many static methods. if I cache it in instance field how I am going to use it in such static methods? could you please clarify more what you mean here?

Right, this would not work everywhere. But it would work for the string manipulation methods on TextInfo or CompareInfo that are probably the most impacted ones.

Feel free to ignore this suggestion if you do not think it is a good idea. It would be useful to pick a few of the leanest paths that the check is getting added to and measure perf impact of this change on them.

lower case char
and above also

will do that

I don't see anything in the coding guidelines about Yoda conditionals but I think we avoid them.

I didn't change this code. also I am not sure why this bad to use?

As you know in native code people often write constant == variable (aka Yoda conditionals) out of habit so that if they mistype constant = variable it will cause a compile error. In C# though there is no implicit conversion in most cases to boolean so eg for integers there is no danger of successfully compiling constant = variable. So in code bases I"ve worked on, typically it's written variable == constant for readability.
Just a nit and I don't see it in the style guidelines although it does seem largely followed here.

I'll fix this one

if want to -> if you want to

will fix that

Is the sole difference between this section and the one above it that this one doesn't limit itself to just ASCII chars, so that we don't call to the OS in invariant mode? Consider a) adding a comment, and b) moving this out into a separate function.

will do that

Rather than having these guards in every function, would it be possible to, for example, implement a special CultureInfo for invariant all in managed code with functions like you have, and then if ICU isn't present / the config switch is thrown, just return that invariant CulutureInfo from anything that can give back a CultureInfo? Maybe that's not feasible, but it seems like it could help to consolidate the logic better.

I have spent some time thinking about how we can implement the whole thing here. I have looked at the alternatives. for CultureInfo we already doing almost your idea mentioned here. but for other classes like CompareInfo/TextInfo I preferred the current implementation. To make it like what you suggested, I'll have to subclass CompareInfo/TextInfo and override all methods and then return such objects to the callers of such properties (like CultureInfo.CompareInfo, CultureInfo.TextInfo) . I dismissed this idea because a) we are not returning CompareInfo as we used to but we are returning a subclass of CompareInfo b) I was trying to avoid the unknown perf cost c) this will restrict us if we decided to expose a constructors for such classes in the future d) we'll have to use most of the existing code in CompareInfo anyway. we can refactor the code for this one though.

is it necessary to duplicate this block? can't the condition above be changed from
while (length != 0 && (*a <= 0x80) && (*b <= 0x80))
to
while (length != 0 && ((*a <= 0x80) && (*b <= 0x80)) || CultureData.InvariantMode)

I didn't do that for the perf reason. this will add extra check with every character.

What about

             byte comparator = CultureData.InvariantMode ? 0xFF : 0x80;
		 
                 while (length != 0 && (*a <= comparator) && (*b <= comparator))
                 {

that would avoid the duplication al,so.

Or a separate function as @stephentoub suggests. It's just confusing two have two blocks like this IMO.

I think this is a good idea. I'll apply it. I'll use 0xFFFF (not 0xFF)

Yes thanks I meant 0xFFFF :) ... this should get a good comment, though, as I had to think about it.

Same here

private?

Same here

This can be readonly

Or even internal static bool Invariant { get; } = GetGlobalizationInvariantMode();