Skip to content
This repository was archived by the owner on Aug 2, 2023. It is now read-only.

Faster IndexOfVectorized#1231

Merged
KrzysztofCwalina merged 1 commit into
dotnet:masterfrom
benaadams:faster-indexof
Feb 21, 2017
Merged

Faster IndexOfVectorized#1231
KrzysztofCwalina merged 1 commit into
dotnet:masterfrom
benaadams:faster-indexof

Conversation

@benaadams
Copy link
Copy Markdown
Member

@benaadams benaadams commented Feb 21, 2017

from dotnet/corefx#16222

Not quite sure how to interpret the benchmark results (from corefxlab\scripts\PerfHarness) but they suggest is x4.5 - x6 faster than .IndexOf for everything except length = 0

Likely want to verify it by someone who understands how the benchmarks work; might all be the same length and 0 is higher as its jitting

@KrzysztofCwalina
Copy link
Copy Markdown
Member

If you run the harness till the end (as opposed to stopping it after some tests pass), the harness will analyze the raw file and create another file with relatively nicely tallied results.

@KrzysztofCwalina
Copy link
Copy Markdown
Member

I ran the benchmark with your changes.
Before: 0.113638251
After: 0.075090739

Nice improvement!

@KrzysztofCwalina
Copy link
Copy Markdown
Member

I added some tests for different indexes. Here are the results:

With your changes:

Test Name Average
IndexOfBench.SpanIndexOf(at: 1000) 0.367757911
IndexOfBench.SpanIndexOf(at: 30) 0.017418343
IndexOfBench.SpanIndexOf(at: 0) 0.003753878
IndexOfBench.VectorizedIndexOf(at: 1000) 0.084792347
IndexOfBench.VectorizedIndexOf(at: 30) 0.017610471
IndexOfBench.VectorizedIndexOf(at: 0) 0.016239759

Without your changes:

Test Name Average
IndexOfBench.VectorizedIndexOf(at: 1000) 0.174605217
IndexOfBench.VectorizedIndexOf(at: 30) 0.092288951
IndexOfBench.VectorizedIndexOf(at: 0) 0.084601138

Nice improvement. I will merge the changes.
Thanks!!

@benaadams
Copy link
Copy Markdown
Member Author

What's it like on the early numbers? (non-zero) compared to regular IndexOf?

public static int IndexOfVectorized(this ReadOnlySpan<byte> buffer, byte value)
public unsafe static int IndexOfVectorized(this ReadOnlySpan<byte> buffer, byte value)
{
Debug.Assert(s_longSize == 4 || s_longSize == 2);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This assert is removed in above (Span overload).

var byteSize = s_byteSize;
fixed (byte* pHaystack = &buffer.DangerousGetPinnableReference())
{
var haystack = pHaystack;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you share code between Span and ReadOnlySpan implementations after getting the pointer?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes they are identical

}

var byteSize = s_byteSize;
fixed (byte* pHaystack = &buffer.DangerousGetPinnableReference())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

haystack is a unorganized pile. Span is an ordered list. I think the parameter name needs to change :-)

@KrzysztofCwalina
Copy link
Copy Markdown
Member

KrzysztofCwalina commented Feb 21, 2017

Test Name Average
IndexOfBench.SpanIndexOf(at: 1000) 0.3853
IndexOfBench.VectorizedIndexOf(at: 1000) 0.0839

IndexOfBench.SpanIndexOf(at: 30) 0.0168
IndexOfBench.VectorizedIndexOf(at: 30) 0.0168

IndexOfBench.SpanIndexOf(at: 16) 0.0114
IndexOfBench.VectorizedIndexOf(at: 16) 0.0170

IndexOfBench.SpanIndexOf(at: 8) 0.0077
IndexOfBench.VectorizedIndexOf(at: 8) 0.0168

IndexOfBench.SpanIndexOf(at: 4) 0.0057
IndexOfBench.VectorizedIndexOf(at: 4) 0.0161

IndexOfBench.SpanIndexOf(at: 2) 0.0045
IndexOfBench.VectorizedIndexOf(at: 2) 0.0169

IndexOfBench.SpanIndexOf(at: 1) 0.0045
IndexOfBench.VectorizedIndexOf(at: 1) 0.0162

IndexOfBench.SpanIndexOf(at: 0) 0.0041
IndexOfBench.VectorizedIndexOf(at: 0) 0.0161

@KrzysztofCwalina
Copy link
Copy Markdown
Member

I am going to merge this and then we can cleanup depending what goes or not into Span directly.

@KrzysztofCwalina KrzysztofCwalina merged commit 76eeca7 into dotnet:master Feb 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants