search

al8n / skl

A lock-free thread-safe arena based Skiplist impelementation for building memtable.
Apache License 2.0
41 stars 5 forks source link

More fixes of insufficient atomics, and make most of the test suite runnable under miri #31

Closed JoJoDeveloping closed 4 weeks ago

JoJoDeveloping commented 1 month ago

Hi there, here are some more bugs similar to these of #30.

Additionally, I added some cfg(miri)s to make the test suite run under miri in a reasonable time.

The reason I am doing all of this is that I noticed that this crate is incompatible with Tree Borrows (TB), a new aliasing model for Rust and a potential replacement of Stacked Borrows. While TB was designed to be more lenient than SB, it also gets rid of some dirty hacks, and these unfortunately were relied on this crate (along with a few others which I am doing my best to fix now).

Or, to be more precise: They break rarena-allocator, which is a dependency of this crate, but the breakage only shows up in the tests in this crate. Fortunately, that crate is another crate of yours.

Unfortunately, while I have fixed the TB errors there, the last published version of rarena-allocator is quite different from the development HEAD (and the version depended on by skl), so I am not sure how to best contribute them. Advice on how to proceed is appreciated, if you have any questions about Tree Borrows or why these changes are necessary, feel free to ask.

al8n commented 1 month ago

Hi, thank you so much!

Regarding the rarena-allocator, the upcoming skl version 0.14.0 (#29) will be based on the changes introduced in the rarena-allocator's pull request (https://github.com/al8n/rarena/pull/8). Therefore, I would like to suggest fix any miri bugs based on https://github.com/al8n/rarena/pull/8.

JoJoDeveloping commented 1 month ago

Wow, that's a lot of change. I'll aim my changes on top of these branches.

al8n commented 1 month ago

I just cleaned up TB errors in rarena-allocator and publish the new version of rarena-allocator. I also ddded CI jobs to test TB in #29, and found more TB errors in this project which not related to rarena-allocator, https://github.com/al8n/skl/actions/runs/10621035558/job/29442139071

JoJoDeveloping commented 1 month ago

Nice. I think the error looks familiar, but of course it might just be a different thing at a similar place. I'll have a look at it after the weekend tho. Thanks for getting things published :)

al8n commented 1 month ago

Nice. I think the error looks familiar, but of course it might just be a different thing at a similar place. I'll have a look at it after the weekend tho. Thanks for getting things published :)

Thanks!

al8n commented 4 weeks ago

I am merging this PR and creating a new issue, #34 for tree borrow stuff.