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

Streamline keys and elems #158

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

treeowl
Copy link
Collaborator

@treeowl treeowl commented Jun 13, 2017

Make sure keys and elems won't build silly pairs and
thunks to deconstruct them.

Make sure `keys` and `elems` won't build silly pairs and
thunks to deconstruct them.
@phadej
Copy link
Contributor

phadej commented Jun 14, 2017

Isn't a guarantee that map . build should fuse? (toList is defined in terms of build)?

The current implementation is way simpler to understand,

@sjakobi
Copy link
Member

sjakobi commented Jun 18, 2020

It would be nice to get some numbers that demonstrate the usefulness of this change.

@treeowl
Copy link
Collaborator Author

treeowl commented Jun 18, 2020

Ah, good questions. We need to look at the Core and see if the pairs and selector thunks are eliminated by fusion. @sjakobi, I've never had a clue of the correct way to ask Cabal to ask GHC to give me -ddump-simpl for a module. I've always just hacked around by running bare GHC. There must be a right way to do this. Do you know it?

@sjakobi
Copy link
Member

sjakobi commented Jun 18, 2020

I've never had a clue of the correct way to ask Cabal to ask GHC to give me -ddump-simpl for a module. I've always just hacked around by running bare GHC. There must be a right way to do this. Do you know it?

What I usually do is insert a OPTIONS_GHC pragma in the relevant module and just cabal build. I usually copy the one from this blog post: https://www.joachim-breitner.de/blog/763-Faster_Winter_5__Eta-Expanding_ReaderT 😄

@sjakobi
Copy link
Member

sjakobi commented Jun 24, 2020

I'll mark this as a draft to clarify that we need benchmark results and/or insights regarding the Core before we can properly consider a merge.

@sjakobi sjakobi marked this pull request as draft June 24, 2020 11:20
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.

3 participants