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

Use uint64_t instead of size_t? #5

Open
rokups opened this issue May 10, 2017 · 5 comments
Open

Use uint64_t instead of size_t? #5

rokups opened this issue May 10, 2017 · 5 comments

Comments

@rokups
Copy link

rokups commented May 10, 2017

Code is using size_t in various places assuming it is at least 64 bit in size. This is not always true. Any chance we could get this more 32bit-friendly?

@jswigart
Copy link

I noticed this too in the log2 and next_power_of_two

which emits

Warning C4293 '>>': shift count negative or too big, undefined behavior

with the 32 bit shifts

definately seems like it warrants a fix

@fallenworld1
Copy link

fallenworld1 commented Mar 19, 2020

these maps don't work on 32bit systems indeed. Replacing size_t with uint64_t helped.

@saierd
Copy link

saierd commented Mar 20, 2020

Also see #18 for 32 bit support.

@shaqiu
Copy link

shaqiu commented May 13, 2024

My native C++ code in an Android app uses this repo's unordered_map to replace std::unordered_map. It performs well on armeabiv8, resulting in a 50% reduction in duplicate entries for a dependency that relies on the hash table. However, when I prepared for gray-box testing by coincidentally switching to armeabiv7 (which is also a required supported version for the project) and tried to compile, it failed at prime_list. Initially, I thought it was a minor issue and changed size_t to llu suffix corresponding to long long unsigned int for array elements, and the compilation passed. However, upon running, it crashed. Then I came across this thread in the issues section, and it was so timely. Next, I'll see if there are any low-cost fixes available. If the author is still maintaining it, could you please fix these issues as soon as possible and also support 32-bit compilation? Thank you!

Below is the original text in Chinese Simplified:
中文原文(🇨🇳同胞们看这条):我的一个安卓app的native c++在引用这个repo的unordered_map来替换std::unordered_map时,armeabiv8表现良好,使得一个依赖hash table的去重下降了50%,然而当我整理提交,准备发灰度测试时,机缘巧合的切换到了armeabiv7(也是项目需要支持的版本)时,编译失败在了prime_list。开始我只是以为是一般的小问题,将size_t改为数组元素的llu后缀对应的long long unsigned int后,编译通过。运行,结果发生了崩溃,然后我就在issues版本看到这个帖子,真是太及时了。下面我会看看有没有低成本的修复方法,然后作者如果还维护的,可以尽快fix这些问题,也支持下32bit编译,好吗?

@shaqiu
Copy link

shaqiu commented May 13, 2024

Also see #18 for 32 bit support.

I've just tried this branch (https://github.com/zertyz/flat_hash_map/tree/master), and it still didn't work for my project, pass compilation for armeabiv7 32bit, yet crashes still take place on using this modified flat hash map.

For those trapped in the similar situation, I've tried hopscotch(https://github.com/Tessil/hopscotch-map), yet it shows little difference on my project. So following this, I rolled back to tsl::robin_map and it worked, unlike my last attempt (it showed little difference probabally due to my error-prone AB comparison approaches), the duplication removal job showed 30% drop in time consumption.

So, you can use robin_map(https://github.com/Tessil/robin-map) as an ready to ship alternative, which is 32bit friendly as well as available for 64bit platforms.

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

No branches or pull requests

5 participants