-
Notifications
You must be signed in to change notification settings - Fork 506
Consuming Microsoft.NETCore.Native package, which brings in libSystem… #2592
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,12 +174,12 @@ private unsafe int IndexOfCore(string source, string target, int startIndex, int | |
| { | ||
| return IndexOfOrdinal(source, target, startIndex, count, ignoreCase: false); | ||
| } | ||
|
|
||
| #if CORECLR | ||
| if (_isAsciiEqualityOrdinal && CanUseAsciiOrdinalForOptions(options) && source.IsFastSort() && target.IsFastSort()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why removed this code? my understanding this can still work.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we should keep the exact code we have in coreclr under \mscorlib\corefx\System\Globalization. right?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we have limitation in corert for keeping such code?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The IsFastSort, etc. bits are not cached in object pre-header in CoreRT. I agree we should keep the CoreCLR specific code under
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do. |
||
| { | ||
| return IndexOf(source, target, startIndex, count, GetOrdinalCompareOptions(options)); | ||
| } | ||
|
|
||
| #endif | ||
| fixed (char* pSource = source) | ||
| { | ||
| int index = Interop.GlobalizationInterop.IndexOf(_sortHandle, target, target.Length, pSource + startIndex, count, options); | ||
|
|
@@ -204,10 +204,12 @@ private unsafe int LastIndexOfCore(string source, string target, int startIndex, | |
| return LastIndexOfOrdinal(source, target, startIndex, count, ignoreCase: false); | ||
| } | ||
|
|
||
| #if CORECLR | ||
| if (_isAsciiEqualityOrdinal && CanUseAsciiOrdinalForOptions(options) && source.IsFastSort() && target.IsFastSort()) | ||
| { | ||
| return LastIndexOf(source, target, startIndex, count, GetOrdinalCompareOptions(options)); | ||
| } | ||
| #endif | ||
|
|
||
| // startIndex is the index into source where we start search backwards from. leftStartIndex is the index into source | ||
| // of the start of the string that is count characters away from startIndex. | ||
|
|
@@ -227,10 +229,12 @@ private bool StartsWith(string source, string prefix, CompareOptions options) | |
| Debug.Assert(!string.IsNullOrEmpty(prefix)); | ||
| Debug.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0); | ||
|
|
||
| #if CORECLR | ||
| if (_isAsciiEqualityOrdinal && CanUseAsciiOrdinalForOptions(options) && source.IsFastSort() && prefix.IsFastSort()) | ||
| { | ||
| return IsPrefix(source, prefix, GetOrdinalCompareOptions(options)); | ||
| } | ||
| #endif | ||
|
|
||
| return Interop.GlobalizationInterop.StartsWith(_sortHandle, prefix, prefix.Length, source, source.Length, options); | ||
| } | ||
|
|
@@ -241,10 +245,12 @@ private bool EndsWith(string source, string suffix, CompareOptions options) | |
| Debug.Assert(!string.IsNullOrEmpty(suffix)); | ||
| Debug.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0); | ||
|
|
||
| #if CORECLR | ||
| if (_isAsciiEqualityOrdinal && CanUseAsciiOrdinalForOptions(options) && source.IsFastSort() && suffix.IsFastSort()) | ||
| { | ||
| return IsSuffix(source, suffix, GetOrdinalCompareOptions(options)); | ||
| } | ||
| #endif | ||
|
|
||
| return Interop.GlobalizationInterop.EndsWith(_sortHandle, suffix, suffix.Length, source, source.Length, options); | ||
| } | ||
|
|
@@ -256,13 +262,13 @@ private unsafe SortKey CreateSortKey(String source, CompareOptions options) | |
|
|
||
| if ((options & ValidSortkeyCtorMaskOffFlags) != 0) | ||
| { | ||
| throw new ArgumentException(Environment.GetResourceString("Argument_InvalidFlag"), nameof(options)); | ||
| throw new ArgumentException(SR.Argument_InvalidFlag, nameof(options)); | ||
| } | ||
|
|
||
| byte [] keyData; | ||
| if (source.Length == 0) | ||
| { | ||
| keyData = EmptyArray<Byte>.Value; | ||
| keyData = Array.Empty<Byte>(); | ||
| } | ||
| else | ||
| { | ||
|
|
@@ -348,9 +354,35 @@ internal unsafe int GetHashCodeOfStringCore(string source, CompareOptions option | |
| } | ||
| } | ||
|
|
||
| [DllImport(JitHelpers.QCall)] | ||
| [SuppressUnmanagedCodeSecurity] | ||
| private static unsafe extern int InternalHashSortKey(byte* sortKey, int sortKeyLength, [MarshalAs(UnmanagedType.Bool)] bool forceRandomizedHashing, long additionalEntropy); | ||
| private static unsafe int InternalHashSortKey(byte* sortKey, int sortKeyLength, bool forceRandomizedHashing, long additionalEntropy) | ||
| { | ||
| if (forceRandomizedHashing || additionalEntropy != 0) | ||
| { | ||
| // TODO: Random hashing is yet to be done | ||
| // Active Issue: https://github.com/dotnet/corert/issues/2588 | ||
| throw new NotImplementedException(); | ||
| } | ||
| else | ||
| { | ||
| int hash1 = 5381; | ||
| int hash2 = hash1; | ||
| if (sortKeyLength == 0) | ||
| { | ||
| return 0; | ||
| } | ||
| if (sortKeyLength == 1) | ||
| { | ||
| return (((hash1 << 5) + hash1) ^ sortKey[0]) + (hash2 * 1566083941); | ||
| } | ||
|
|
||
| for (int i = 0; i < (sortKeyLength & ~1); i += 2) | ||
| { | ||
| hash1 = ((hash1 << 5) + hash1) ^ sortKey[i]; | ||
| hash2 = ((hash2 << 5) + hash2) ^ sortKey[i+1]; | ||
| } | ||
| return hash1 + (hash2 * 1566083941); | ||
| } | ||
| } | ||
|
|
||
| private static CompareOptions GetOrdinalCompareOptions(CompareOptions options) | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -144,7 +144,6 @@ | |
| <Compile Include="System\Runtime\InteropServices\Marshal.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\MarshalAdapter.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\MarshalImpl.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\MarshalAsAttribute.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\MarshalDirectiveException.cs" /> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add forwarders for these from System.Private.Interop to System.Private.CoreLib so that the existing ProjectN corefx drop does not break?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do. |
||
| <Compile Include="System\Runtime\InteropServices\MissingInteropDataException.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\OptionalAttribute.cs" /> | ||
|
|
@@ -153,9 +152,7 @@ | |
| <Compile Include="System\Runtime\InteropServices\SafeArrayTypeMismatchException.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\SEHException.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\TypeIdentifierAttribute.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\UnmanagedType.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\UnknownWrapper.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\VarEnum.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\Variant.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\VariantWrapper.cs" /> | ||
| <Compile Include="System\Runtime\InteropServices\ComWeakReferenceHelpers.cs" /> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have this inconsistency in library naming? We have System.Native.a, but libSystem.Globalization.Native.a. On CoreCLR, none of the shared libraries that are named with System.XXXX have the lib prefix, so it seems strange to have it for some of the static ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we should fix the names to be consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't libXXX more adopted naming convention in the industry? Why did we name those libs as such on CoreCLR in the first place? Shall we fix libSystem.Globalization.Native.a or System.Native.a ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have made this choice a long time ago so that the shared library name matches the corresponding managed assembly. You can see the quite long discussion here: dotnet/coreclr#745
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pointer @janvorli. I'll update libSystem.Globalization.Native.a name