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

Implement unordered container support, xhash no floating point #6

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jxy-s
Copy link
Owner

@jxy-s jxy-s commented Sep 18, 2021

This PR will add support for unordered containers (unordered_map, unordered_multimap, unordered_set, and unordered_multiset).

To do this I must patch xhash from the MSVC STL to eliminate the floating point arithmetic. I do this by copying xhash out of the MSVC STL and applying changes under the _XHASH_NO_FLOATING_POINT define. I then directly include the patched version through jxy/unordered_map.hpp and jxy/unordered_set.hpp defining _XHASH_NO_FLOATING_POINT to 1 before doing so.

The solution here isn't ideal. I would much rather PR this into MSVC directly. I have a branch that does this for MSVC (microsoft/STL@main...jxy-s:xhash-no-floating-point) and I will engage with the MSVC community on it. However, the likelihood of this happening in a timely manner (if at all) is slim - it is, understandably, simply not a priority for them.

@jxy-s jxy-s force-pushed the jxy-unordered-container-support branch from 6acdc9f to 0ac1d3b Compare September 18, 2021 21:35
@jxy-s jxy-s force-pushed the jxy-unordered-container-support branch from 68385d4 to 494789e Compare September 19, 2021 02:32
mrexodia added a commit to thesecretclub/riscy-business that referenced this pull request Jan 12, 2024
vector, string and unordered_map work, mainly thanks to jxy-s/stlkrn#6
mrexodia added a commit to mrexodia/STL that referenced this pull request Jan 12, 2024
mrexodia added a commit to mrexodia/STL that referenced this pull request Jan 12, 2024
@frederick-vs-ja
Copy link

MSVC STL's hash containers are currently implemented in a bad manner - e.g. the are base on doubly linked lists while singly linked lists are preferred. Unfortunately, we can't achieve improvement in representation at this moment due to ABI compatibility.

It seems to me that the switching to return uint64_t for load_factor() is already ABI-breaking (and... non-conforming). So given ABI compatibility with the existing MSVC STL is not (and probably shouldn't be) concerned, I think it's better not to resue MSVC STL's <xhash> - or at least to resuse it with more changes.

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.

None yet

2 participants