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

Replace dashmap with scc #109

Merged
merged 5 commits into from
Mar 30, 2024
Merged

Replace dashmap with scc #109

merged 5 commits into from
Mar 30, 2024

Conversation

palfrey
Copy link
Owner

@palfrey palfrey commented Mar 30, 2024

Also add a lot of debug (primarily around start of test, etc) which should all default to off

Mostly because of https://dev.to/acter/beware-of-the-dashmap-deadlock-lij but also wondering if this will solve the periodic test failures that I haven't previously been able to debug.

@palfrey palfrey merged commit 2389412 into main Mar 30, 2024
7 checks passed
@palfrey palfrey deleted the remove-dashmap branch March 30, 2024 23:33
@decathorpe
Copy link

I can add another data point here. In order to update the Fedora Linux package for the serial_test crate we had to package the scc crate too, and I ended up having to disable tests (for now) - about 50% of test runs failed due to non-deterministic test failures or crashes due to "unreachable code reached" or "out of bounds array access".

Most issues looked like race conditions - possibly due to incorrect usage of atomics, since none of the failures happened on x86_64 (which has stronger memory ordering guarantees than other architectures - even using atomics wrong can appear to "work" correctly). Incorrect usage of atomics for memory accesses can lead to unsoundness and / or correctness issues, as can be seen from the test logs in the issue I opened:

wvwwvwwv/scalable-concurrent-containers#153

@palfrey
Copy link
Owner Author

palfrey commented Aug 3, 2024

That's interesting. So part of the serial_test CI runs on a macos-latest image, which as I've just found out is now an arm64 machine, and that appears to work fine. Looking at those test failures, they appear to mostly either be x86-specific tests (e.g. the sizeof bits) or ones on TreeIndex which serial_test doesn't use.

In other words, I think for most users on arm64 (and hopefully powerpc64le) they can just use scc as part of serial_test and it just works, and this is mainly an issue around packaging right? Also I'm seeing good responses on that ticket from the scc maintainer, so hopefully that gets fixed soon anyways.

If that can't get resolved in the coming weeks, I might have to look at another library eventually, but I've been happy with scc so far.

@decathorpe
Copy link

Sounds like a good plan! Just wanted to let you know about possibly unintended consequences caused by the switch from dashmap to scc.

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.

2 participants