Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Benchmark for FindIndices #272

Merged
merged 6 commits into from
Sep 10, 2020
Merged

Benchmark for FindIndices #272

merged 6 commits into from
Sep 10, 2020

Conversation

archaephyrryx
Copy link
Contributor

Adds a benchmark suite for measuring the performance of Data.ByteString.findIndices relative to Data.ByteString.findIndex (and the related Data.ByteString.elemIndices and Data.ByteString.elemIndex) over inlinable and uninlinable equality tests.

For comparing performance of relevant functions before and after changes implemented in #270

Adds separate benchmark suite for comparing the relative performance
of findIndex, findIndices, elemIndex, and elemIndices over first
occurence and second occurence in sparse strict bytestring

Also includes benchmark for performance of uninlined versus inlined
equality test as predicate for findIndex and findIndices
bench/bench-bytestring.cabal Show resolved Hide resolved
bench/bench-bytestring.cabal Outdated Show resolved Hide resolved
bench/BenchIndices.hs Outdated Show resolved Hide resolved
bench/BenchIndices.hs Outdated Show resolved Hide resolved
Fixes issues with benchmark cabal file by re-including C source files
and headers; also removes leftover comment regarding blaze builder

Amends inaccurate description of BenchIndices module

Removes explicit NOINLINE pragma for the `absurdlong` test string
Removes import of GHC.Word.eqWord8 from BenchIndices,
as GHC.Word does not export eqWord8 in GHC 7.10.3.

Reimplements nilEq in terms of (==)
bench/BenchIndices.hs Outdated Show resolved Hide resolved
@archaephyrryx
Copy link
Contributor Author

The Travis checks appear to have been reported as failing due to one of the checks getting terminated by a power-off before it could even begin building. The change I made was only in haddock so there is no conceivable way it could break any checks. Unless I push again and thereby cause a rebuild, or someone with write access manually forces a recheck, please consider the latest commit (52b6e44) as passing all checks, as the previous 43e4d41 did.

Copy link
Contributor

@Bodigrim Bodigrim left a comment

Choose a reason for hiding this comment

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

This is all good and well-thought stuff, but I'd prefer less indirections. Names of many functions are longer than their definitions, and, taking into account type signatures and INLINE pragmas, signal-to-noise ratio is not ideal.

Could you please inline most of one-line-long definitions?

bench/BenchIndices.hs Show resolved Hide resolved
bench/BenchIndices.hs Outdated Show resolved Hide resolved
bench/BenchIndices.hs Outdated Show resolved Hide resolved
Fix dangling `in` from removed `let` binding

Co-authored-by: Viktor Dukhovni <viktor1ghub@dukhovni.org>
Copy link
Contributor

@vdukhovni vdukhovni left a comment

Choose a reason for hiding this comment

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

LGTM

@Bodigrim Bodigrim added this to the 0.11.0.0 milestone Sep 9, 2020
@Bodigrim Bodigrim merged commit 7ff2248 into haskell:master Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants