Skip to content

Conversation

@MihaZupan
Copy link
Member

Also solves an edge-case bug for byte overloads on ARM64 where we'd get false positives (negatives for Except) when haystack values were exactly 128 above a value in the needle.
Closes #78718

Before

Method Length Needle Mean Error
IndexOfAny128_Char 100000 AlphaNumeric 18.27 us 0.035 us
IndexOfAny128_Byte 100000 AlphaNumeric 11.37 us 0.032 us

After

Method Length Needle Mean Error
IndexOfAny128_Char 100000 AlphaNumeric 16.363 us 0.3711 us
IndexOfAny128_Byte 100000 AlphaNumeric 9.621 us 0.0884 us

Also solves an edge-case bug for byte overloads on ARM64 where we'd get false positives (negatives for Except) when haystack values were exactly 128 above a value in the needle
@MihaZupan MihaZupan added this to the 8.0.0 milestone Nov 23, 2022
@ghost ghost assigned MihaZupan Nov 23, 2022
@ghost
Copy link

ghost commented Nov 23, 2022

Tagging subscribers to this area: @dotnet/area-system-memory
See info in area-owners.md if you want to be subscribed.

Issue Details

Also solves an edge-case bug for byte overloads on ARM64 where we'd get false positives (negatives for Except) when haystack values were exactly 128 above a value in the needle.
Closes #78718

Before

Method Length Needle Mean Error
IndexOfAny128_Char 100000 AlphaNumeric 18.27 us 0.035 us
IndexOfAny128_Byte 100000 AlphaNumeric 11.37 us 0.032 us

After

Method Length Needle Mean Error
IndexOfAny128_Char 100000 AlphaNumeric 16.363 us 0.3711 us
IndexOfAny128_Byte 100000 AlphaNumeric 9.621 us 0.0884 us
Author: MihaZupan
Assignees: -
Labels:

area-System.Memory

Milestone: 8.0.0

haystackWithOffsetNeedle[i] = (byte)(128 + needleValues[i % needleValues.Length]);
}

Assert.Equal(-1, haystackWithOffsetNeedle.IndexOfAny(needle));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test would blow up here on ARM before this change

Vector128<byte> ascii = Sse2.IsSupported
? Sse2.PackSignedSaturate(ascii0, ascii1).AsByte()
: AdvSimd.Arm64.UnzipEven(ascii0.AsByte(), ascii1.AsByte());
Vector128<byte> ascii = Sse2.PackSignedSaturate(ascii0, ascii1).AsByte();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: does the ascii local temporary help with anything?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, I just found it a bit more readable when initially writing this logic

@MihaZupan
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FileNameStar_NeedsEncoding_EncodedAndDecodedCorrectly test is failing in CI

4 participants