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

Depend on primitive #206

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

andrewthad
Copy link
Contributor

Work in progress.

Currently, unordered-containers includes a module Data.HashMap.Array that is nearly a copy of Data.Primitive.SmallArray. The module from unordered-containers also contains some other goodies for doing things like inserting at an index, deleting an element, etc. (having these in primitive wouldn't make sense). This PR changes unordered-containers to use types and functions from primitive where possible.

The initial commit starts by renaming the functions and types from Data.HashMap.Array to match the names they are given in Data.Primitive.SmallArray.

Other things that need to happen for this to be finished:

  • I need to actually substitute use the functions from primitive.
  • A new version of primitive needs to be released to hackage. This should be happening soon.
  • I need to benchmark on a headless computer to ensure that there are no performance regressions.

…atch names of functions from Data.Primitive.SmallArray. For the copying functions, this also requires flipping the order of the arguments.
@andrewthad
Copy link
Contributor Author

I just pushed the changes to switch over to SmallArray from primitive. This won't build on travis yet since it requires changes to primitive that have not yet been released to hackage. I'm currently building it locally by writing a cabal.local.project with:

packages: . ../primitive

It passes the test suite.

@andrewthad
Copy link
Contributor Author

Here are some benchmarks: https://gist.github.com/andrewthad/d387fee3231ebaa58161c009cb1703a5.
This was the result of running the benchmark suite on my computer booted into run level 3 (no windowing system was running).

The first column is the benchmark name. The second is the delta between the means (status quo minus primitive). A negative number signifies that it got faster (or more likely it's just because of a little bit of system noise). A positive number signifies that it got slower (or again because of noise). I'm still trying to figure out how to draw a meaningful conclusion from this information.

@emilypi
Copy link
Member

emilypi commented May 29, 2020

Thanks @andrewthad! This is a significant amount of work and I'd love to get to reviewing it at some point when we've cleared out some higher priority tasks. David now has some help, so we'll probably be able to consider this relatively soon.

@sjakobi sjakobi linked an issue May 31, 2020 that may be closed by this pull request
@sjakobi
Copy link
Member

sjakobi commented Jun 18, 2020

Here are some benchmarks:

FWIW, criterion-compare is handy for analyzing differences between benchmark runs.

@sjakobi sjakobi marked this pull request as draft June 18, 2020 01:43
@treeowl
Copy link
Collaborator

treeowl commented Jun 18, 2020

I remember @tibbe being concerned about how the PrimMonad business could affect specialization, inlining, etc. GHC has gotten somewhat more aggressive about specialization in some cases; I don't know if those concerns remain valid.

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.

Consider depending on primitive
4 participants