RFC for histogram CPU implementation

[danhoeflinger]

Adds an RFC for histogram CPU implementation.

[akukanov]

Overall, this all sounds good enough for the "proposed" stage, where it's expected that some details are unknown and need to be determined. I am happy to approve it but will wait for a few days in case @danhoeflinger wants to update the document with some follow-up thoughts on the discussion.

[danhoeflinger]

One question I have for the group...
If we know that a serial implementation will provide better performance up to some threshold (perhaps dependent on num bins, num threads, num input elements), can / should we dispatch instead to a serial implementation?

From my reading, it seems the answer is probably no. Execution policies have semantic meaning, and par / par_unseq do not simply mean "provide the fastest version" even if that is what the users probably want.

[mmichel11]

One question I have for the group... If we know that a serial implementation will provide better performance up to some threshold (perhaps dependent on num bins, num threads, num input elements), can / should we dispatch instead to a serial implementation?

From my reading, it seems the answer is probably no. Execution policies have semantic meaning, and par / par_unseq do not simply mean "provide the fastest version" even if that is what the users probably want.

I agree that we should honor the user's request for a specific policy as opposed to using the serial implementation until some empirically determined cutoff point. I also imagine that the exact cutoff point where the parallel version performs better can highly vary dependent on a user's hardware setup and giving them the freedom to manually choose when to make the switch from the serial to parallel version may result in better performance than any generic decisions we could make.

[mmichel11]

I've taken another pass through the document. A single question regarding how technical we want to get when explaining the algorithm.

The RFC looks ready to me.

[akukanov]

One question I have for the group...
If we know that a serial implementation will provide better performance up to some threshold (perhaps dependent on num bins, num threads, num input elements), can / should we dispatch instead to a serial implementation?

From my reading, it seems the answer is probably no. Execution policies have semantic meaning, and par / par_unseq do not simply mean "provide the fastest version" even if that is what the users probably want.

I believe yes, we can. Generally, a serial implementation is correct for parallel execution policies, so it's more of a QoI question whether to parallelize or not. While the policies do not mean "provide the fastest version", they do not mean "always use multiple threads" either. It's a permission to use multiple threads, but not an obligation.

[MikeDvorskiy]

The second sentence may be omitted.
Based on the first sentence we can conclude that "OneDPL does not directly use any intrinsics..."

[danhoeflinger]

applied.

[MikeDvorskiy]

But we can calculate the bin indexes for the input data in SIMD manner.
After that we can process the result in a serial loop.
No?

[danhoeflinger]

This is applicable only for the even binned case. Without using intrinsic operations, we must do this with omp simd and the ordered structured block. Initial investigation seemed to indicate that this was unsuccessful for generating vectorized code, and my suspicion is that it will not really help anyway. I can revisit this and attempt it, but the intention for now was to omit vectorizations from this first phase.

[danhoeflinger]

For now I'll ask that we leave it as described in the RFC, which gives some understanding of how this can be improved in the future, but starts without vectorization for this phase.
We can add an issue to explore using simd ordered to get some improvement for histogram even, and leave it out for this RFC and the initial PR implementation.

[MikeDvorskiy]

I see.. Ok, lets leave it as described.

[MikeDvorskiy]

I believe yes, we can. Generally, a serial implementation is correct for parallel execution policies, so it's more of a QoI question whether to parallelize or not. While the policies do not mean "provide the fastest version", they do not mean "always use multiple threads" either. It's a permission to use multiple threads, but not an obligation.

Agree with Alexey. Especially since, at least, the host patterns "sort" and "merge" use some thresholds for switching to a serial implementation, depending on number of input elements.
So, probably it makes sense to add a description that the implementation may do fallback to a serial implementation for a "small" number of input elements by performance reasons.

[MikeDvorskiy]

Overall, looks good to me.
Just one comment about adding a description that the implementation may do a fallback to a serial implementation.

[akukanov]

Re-approved.

The questions in my earlier approval are now addressed. The last couple of comments I made do not hold the proposal from landing.

[danhoeflinger]

Waiting for a second approval to merge, so I added the small asks you made in the mean time @akukanov .

[MikeDvorskiy]

LGTM

[danhoeflinger]

Merging, with approvals from @akukanov and @MikeDvorskiy .