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

isPermutationBy is unnecessarily slow #155

Open
treeowl opened this issue Jun 12, 2017 · 8 comments
Open

isPermutationBy is unnecessarily slow #155

treeowl opened this issue Jun 12, 2017 · 8 comments

Comments

@treeowl
Copy link
Collaborator

treeowl commented Jun 12, 2017

Currently, the equality test converts arrays to lists and then uses isPermutationBy to check whether one is a permutation of the other. This seems even more wasteful than necessary. Here's a general plan for performing the test more quickly:

  1. Check that the lengths match
  2. Thaw one of the arrays (safely). This should be less than half the allocation required to convert a single array to a list.
  3. Walk over the unthawed array. For each element:
    a. Search for the matching one in the thawed array.
    b. If it's found, (conceptually) perform an element swap to align it with its match. In fact, the element itself can be discarded, leaving an undefined element aligned with the match.
@tibbe
Copy link
Collaborator

tibbe commented Jun 13, 2017

I'm open for pull requests! (Also I probably need a co-maintainer at some point, given that I'm not very involved with Haskell anymore).

@treeowl
Copy link
Collaborator Author

treeowl commented Jun 13, 2017 via email

@tibbe
Copy link
Collaborator

tibbe commented Jun 13, 2017

Yes. Let me try to put some developer docs together the next few days.

@treeowl
Copy link
Collaborator Author

treeowl commented Jun 13, 2017 via email

@phadej
Copy link
Contributor

phadej commented Jun 13, 2017

@treeowl out of curiosity, do you have a real world case where there is a lot of collisions (i.e. Collision nodes are e.g. >= 3 in length)? Or is it more "let's make it fast, because it seems to be slow". I'm 👍 for optimising, but not sure if we can have realistic benchmark (which we should!)

@treeowl
Copy link
Collaborator Author

treeowl commented Jun 13, 2017 via email

@tibbe
Copy link
Collaborator

tibbe commented Jun 16, 2017

@treeowl I've added some initial docs in https://github.com/tibbe/unordered-containers/blob/master/docs/developer-guide.md and added you as a contributor.

Since I'm way to familiar to the code base to know what needs to be documented, please let me know what would be helpful.

@sjakobi
Copy link
Member

sjakobi commented Feb 16, 2022

3. Walk over the unthawed array. For each element:
a. Search for the matching one in the thawed array.
b. If it's found, (conceptually) perform an element swap to align it with its match. In fact, the element itself can be discarded, leaving an undefined element aligned with the match.

For reference: #291 is a similar optimization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants