FindIndices optimized using findIndex and inlining

[archaephyrryx]

Reimplements findIndices to iteratively call findIndex, which
yields an approximate 2x improvement over original implementation
for a simple equality predicate

Adds inline pragma for findIndices, which yields an approximate 10x
improvement for a simple equality predicate

Additionally inlines findIndices functions in
Data.ByteString{.Lazy,}{.Char8,}

resolves #269

[Bodigrim]

Impressive! Could you please add benchmarks?

[Bodigrim]

Could we possibly also add rewrite rules?

{-# RULES
"ByteString specialise findIndex (x ==)" forall x. findIndex (x ==) = elemIndex x
"ByteString specialise findIndex (== x)" forall x. findIndex (== x) = elemIndex x
"ByteString specialise findIndices (x ==)" forall x. findIndices (x ==) = elemIndices x
"ByteString specialise findIndices (== x)" forall x. findIndices (== x) = elemIndices x
  #-}

This would fix haskell-streaming/streaming-bytestring#15 for free.

[archaephyrryx]

Could we possibly also add rewrite rules?

{-# RULES
"ByteString specialise findIndex (x ==)" forall x. findIndex (x ==) = elemIndex x
"ByteString specialise findIndex (== x)" forall x. findIndex (== x) = elemIndex x
"ByteString specialise findIndices (x ==)" forall x. findIndices (x ==) = elemIndices x
"ByteString specialise findIndices (== x)" forall x. findIndices (== x) = elemIndices x
  #-}

I haven't really explored the RULES pragma deeply enough to know for certain, but I am reasonably sure that we would have to be careful to specify an appropriate phase number so that the rule can fire before == is inlined to a type-determined monomorphic equality function. I could at the very least try to see what I can come up with that compiles without "rule may not fire" warnings that produces the appropriate behavior.

I did end up benchmarking the relative performance of findIndex and findIndices before and after the changes (both the reimplementation and the inlining, as separate comparisons), but any benchmark I write will only reflect the current relative performance, and won't indicate a diachronic benchmark of the previous implementation of findIndices against the reimplemented one. I can still include it, but it is perhaps only useful for posterity if the implementation changes again. If you let me know exactly what kind of benchmark you had in mind, I can try to include something appropriately illustrative of the comparison you wanted to make.

[Bodigrim]

Yes, you can use

-- For Data.ByteString.Char8
{-# INLINE [1] findIndices #-}
{-# RULES "findIndices (== x)" forall x. findIndices (`eqChar` x) = elemIndices x #-}

-- For Data.ByteString
{-# INLINE [1] findIndices #-}
{-# RULES "findIndices (== x)" forall x. findIndices (`eqWord8` x) = elemIndices x #-}

Cf.

bytestring/Data/ByteString.hs

Lines 879 to 881 in 155bf8a

	{-# RULES
	"ByteString specialise break (x ==)" forall x.
	break (x `eqWord8`) = breakByte x

and

bytestring/Data/ByteString/Char8.hs

Lines 538 to 540 in 155bf8a

	{-# RULES
	"ByteString specialise break (x==)" forall x.
	break (x `eqChar`) = breakChar x

.

---

Just add a single benchmark for findIndices in a separate commit, ideally before the commit changing its implementation.

[Bodigrim]

While I have stylistic suggestions for benchmarks in #272, I succesfully used them to verify performance improvements claimed in this PR. All looks good to me, and since this is a massive improvement I do not want it to be blocked by bikeshedding in #272.

[Bodigrim]

Thanks, @archaephyrryx!