Array.Parallel - search functions (tryFindIndex,tryFind,tryPick)

[T-Gro]

This is second part of the RFC to expand the Array.Parallel module, based on approved suggestion

This PR adds single-item search functions tryFindIndex,tryFind,tryPick, with the fulfilled property of returning first element (by overall index, not just time-wise) found.
This is accomplished using the Parallel.For function, and it's ability to terminate early (ParallelLoopState.Break()) and retrieve the first index where this occurred via LowestBreakIteration.
Calling .Break() (as opposed to immediate .Stop()) ensures that all indexes lower than the terminating one are still processed, but higher ones will be skipped if not started already.

This was done to ensure the behavior is consistent with the same functions in Array (non-parallel) module.

[vzarytovskii]

Wondering if the APIs should be marked as Experimental, in case if we'll want to change implementation details later.

[T-Gro]

As of now, the usage of [<Experimental>] within Fsharp.Core seems to be related only to language features (e.g. reverse index methods bound to this feature), I do not see a precedence for using it on plain API additions.

That being sad, I have no problem of putting it in.

[vzarytovskii]

As of now, the usage of [<Experimental>] within Fsharp.Core seems to be related only to language features (e.g. reverse index methods bound to this feature), I do not see a precedence for using it on plain API additions.

That being sad, I have no problem of putting it in.

I think we may do it for more complex functions? Like sorting, so we can easily change implementation details of it? Wdyt?

[T-Gro]

I would for sure use it if the public API itself is potentially subject to change (not the case here, yet). That was actually my current view of it - `[] for the existence and shape of the API itself.

For implementation details (e.g. how much threads it uses, in what scenarios it can be harmful compared to non-parallel code) - maybe this attribute is not the best fit, but I guess it's a good indication of something new and to raise awareness.

Last but not least, it's good to have it if we want to have some space of removing it altogether for whatever reason.
(e.g. it will not get used and only increase code size)

=> I am now more inclined of putting it in place, especially for the last reason.

[brianrourkeboll]

Not that it would matter most of the time (especially since this is Array.Parallel and not regular Array, and allocations are probably gonna happen anyway), but given that this is standard library code that probably won't change very often, it might not hurt to avoid the extra allocations here¹²³:

Suggested change

	[<CompiledName("TryFindIndex")>]
	let tryFindIndex predicate (array: _[]) =
	checkNonNull "array" array
	let pResult =
	Parallel.For(
	0,
	array.Length,
	(fun i pState ->
	if predicate array[i] then
	pState.Break())
	)
	pResult.LowestBreakIteration |> Option.ofNullable |> Option.map int
	[<CompiledName("TryFind")>]
	let tryFind predicate (array: _[]) =
	array |> tryFindIndex predicate |> Option.map (fun i -> array[i])
	let inline tryFindIndexAux predicate (array: _ array) =
	checkNonNull (nameof array) array
	let pResult =
	Parallel.For(
	0,
	array.Length,
	(fun i pState ->
	if predicate array[i] then
	pState.Break())
	)
	pResult.LowestBreakIteration
	[<CompiledName("TryFindIndex")>]
	let tryFindIndex predicate (array: _ array) =
	let i = tryFindIndexAux predicate array
	if i.HasValue then Some (int (i.GetValueOrDefault()))
	else None
	[<CompiledName("TryFind")>]
	let tryFind predicate (array: _ array) =
	let i = tryFindIndexAux predicate array
	if i.HasValue then Some array[int (i.GetValueOrDefault()]
	else None

Footnotes

1. 2 for tryFindIndex: one Some from Option.ofNullable and another from Option.map. ↩

2. 3 for tryFind: one Some from Option.ofNullable, another from the first Option.map, and a third from the outer Option.map. ↩

3. Or maybe I've just read too many Stephen Toub PRs 🙃. ↩

[brianrourkeboll]

Someday, we should add a nullArg (nameof arg) codefix :)

Suggested change

	checkNonNull "array" array
	checkNonNull (nameof array) array