Performance improvements of __parallel_find_or + __device_backend_tag

[SergeyKopienko]

In this PR we made some performance improvements:

• some performance improvements for __parallel_find_or + __device_backend_tag for the usage with the __parallel_or_tag.

---

The changes for

• __pattern_any_of(__hetero_tag has been implemented on __parallel_transform_reduce;
• __pattern_find_if(__hetero_tag has been implemented on __parallel_transform_reduce;
  has been extracted to the separate PR Performance improvements of __pattern_any_of, __pattern_find_if #1622
  and rolled back in this PR.

[SergeyKopienko]

I believe we are able to remove this extra __comp(__found_local.load(), __found.load()) call
due it's already implemented in for-loop below.

[SergeyKopienko]

I am going to remove all comments like // Point # after receive some reviews.
It may help to compare differences between some specializations for the __parallel_or_tag and for the __parallel_find_forward_tag, __parallel_find_backward_tag

[SergeyKopienko]

@MikeDvorskiy, how do you think, does it make sense to apply this changes for range's implementations too?

Actually, we should no duplicate the changes. The iterator based __pattern_any_of and range based __pattern_any_of recall a range based corresponding hetero backend pattern: oneapi::dpl::__par_backend_hetero::__parallel_find_or.

So, with right code design perspective you should modify oneapi::dpl::__par_backend_hetero::__parallel_find_or or another hetero backend new find pattern (reduction based f.e).

[MikeDvorskiy]

Essentially, did you "split" __parallel_find_or pattern into two with "OR search logic" and "first/last search logic".
In that cause I would propose to have different names for patterns: __parallel_find_or and __parallel_find_first.

[SergeyKopienko]

@MikeDvorskiy, how about __parallel_find_or and __parallel_find_entry ?

[MikeDvorskiy]

the standard and cppreference uses the "first" word.
As variant
__parallel_find_first
and
__parallel_find_any

[SergeyKopienko]

Fixed: __parallel_find_or has been renamed to __parallel_find_first and __parallel_find_any
And some staff too.

[MikeDvorskiy]

As far as I see __found_local variable is created and accessible just for the current "thread" (work item), it is not placed in SLM or global memory and is not shared between work items. So, there is no concurrent access here and we don't need use any atomic, a simple bool type is enough.

[SergeyKopienko]

In global atomic type bool is absent.
So really it's one of integer types here.

[danhoeflinger]

Assuming we keep this strategy, I'd be in favor of using a different name for the type _AtomicType is a very confusing typename for something which is not being used as an atomic.
I'd prefer to define two aliases for the same type for clarity here if you wanted to use _AtomicType for the global atomic, and have it share a type with the _LocalStatusType.

[MikeDvorskiy]

if a solution is found, the current work item may don't compute __pred any more with "find_or" algorithm perspective. Did you try to skip __pred computation and estimate performance change? I understand that in case of simple predicate such checks might lead to overheads... But in any case, can we get a performance gain here?

[danhoeflinger]

I think that this workaround _ONEDPL_COMPILE_KERNEL is unnecessary here. My understanding of this kernel compilation bundle step is that it is a workaround to prevent very large allocations from occurring on CPU targets which may return very large __max_work_group_size if left unchecked. Since we are not allocating based upon workgroup size, I don't think this is needed in the first place.

If we want some reasonable limit on workgroup size, this can be replaced by a simple "reasonable" maximum workgroup size like 1024 as is done in histogram.

    ::std::size_t __max_wgroup_size = oneapi::dpl::__internal::__max_work_group_size(__exec);
    ::std::uint16_t __work_group_size = ::std::min(::std::size_t(1024), __max_wgroup_size);

I intend to investigate if we can move away from this workaround altogether in oneDPL, but I have not done so yet. For the time being though I'd prefer not to propagate it unless we have some specific motivation for doing so which I am unaware of.

[SergeyKopienko]

@MikeDvorskiy, what do you think about _ONEDPL_COMPILE_KERNEL and etc.?

[MikeDvorskiy]

My understanding was (is) that we want to know "right" maximus work group size, so the all processed values by WG will be fit into SLM memory. It leads to conclusion that it needs just for those kernels where we use SLM.

[danhoeflinger]

Am I correct that this is actually never called in the current PR?

the __parallel_or_tag overload takes __comp as a parameter but discards it without use.

[SergeyKopienko]

You are correct, fixed.

[MikeDvorskiy]

As far as I can see, here __found_local is not atomic, just a bool/int type and each work item computes predicate and "touches" (updates) the global atomic only if the current predicate is true. I guess such approach may be effective if an input sequence has "few" solutions.
I think if an input sequence has many solutions, each time a work item will "touch" global atomic to update. An access to global atomic is very "expensive". I guess we might have a big performance penalty in that case.