Skip to content

Conversation

@sjakobi
Copy link
Member

@sjakobi sjakobi commented May 18, 2022

No description provided.

@sjakobi
Copy link
Member Author

sjakobi commented May 18, 2022

I haven't been able to find any changes in the benchmarks so far. The generated code looks better though.

@treeowl
Copy link
Collaborator

treeowl commented May 18, 2022

I'm a bit surprised GHC doesn't undo this optimization. If the generated code looks better, go for it.

@sjakobi
Copy link
Member Author

sjakobi commented May 18, 2022

I'm a bit surprised GHC doesn't undo this optimization.

Yeah, GHC is pretty eager to generate a branch here. E.g. idx2 = fromEnum (... < ...) would still generate the branch.

@sjakobi sjakobi marked this pull request as ready for review May 18, 2022 17:07
@sjakobi sjakobi merged commit 6a0fed1 into master May 18, 2022
@sjakobi sjakobi deleted the sjakobi/two-tweaks branch May 18, 2022 17:07
@treeowl
Copy link
Collaborator

treeowl commented May 18, 2022

We have

index :: Hash -> Shift -> Int
index w s = fromIntegral $ unsafeShiftR w s .&. subkeyMask

So I think another option would be to shift the subKeyMask left and use the result for both hashes. I dunno if that would be better, worse, or exactly the same.

@sjakobi
Copy link
Member Author

sjakobi commented May 18, 2022

Yes, there are more low-level tweaks that can be applied here. #468 (comment) is related.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants