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.

Sign up for GitHub

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

Jump to bottom

refactor: replace infallible #6568

Merged
merged 8 commits into from
Jul 12, 2022
Merged

refactor: replace infallible #6568

merged 8 commits into from
Jul 12, 2022

Conversation

PsiACE
Copy link
Member

@PsiACE PsiACE commented Jul 11, 2022 •
edited
Loading

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

After this PR:

  • remove useless code in common_base::infallible, they are just some wrappers, we can use parking_lot or std::sync

    • the Mutex in std is likely to perform better and we can use it.

  • for ReentrantMutex & ReentrantMutexGuard in parking_lot

    • ReentrantMutexGuard does not give mutable references to the locked data. Use a RefCell if you need this.
    • but RefCell is !Sync, perhaps we could use threadsafe AtomicRefCell

@vercel
Copy link

vercel bot commented Jul 11, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
databend ⬜️ Ignored (Inspect) Jul 12, 2022 at 3:59AM (UTC)

@mergify mergify bot added the pr-refactor this PR changes the code base without new features or bugfix label Jul 11, 2022
@PsiACE
Copy link
Member Author

PsiACE commented Jul 11, 2022

this PR broke the cluster mode and I'll try to fix it tomorrow.

@PsiACE PsiACE changed the title refactor: use parking_lot to replace infallible, and intro atomic_refcell refactor: replace infallible Jul 11, 2022
@Xuanwo
Copy link
Member

Xuanwo commented Jul 12, 2022 •
edited
Loading

@PsiACE Nice work! You are databend's code guard!

@PsiACE PsiACE marked this pull request as ready for review July 12, 2022 03:35
@PsiACE PsiACE requested a review from zhang2014 July 12, 2022 03:35
zhang2014
zhang2014 approved these changes Jul 12, 2022
View reviewed changes
Copy link
Member

@zhang2014 zhang2014 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job.

@mergify mergify bot merged commit 865773a into databendlabs:main Jul 12, 2022
@PsiACE PsiACE deleted the rm-infallible branch July 12, 2022 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xuanwo Xuanwo Xuanwo approved these changes

@zhang2014 zhang2014 zhang2014 approved these changes

Assignees
No one assigned
Labels
pr-refactor this PR changes the code base without new features or bugfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PsiACE @Xuanwo @zhang2014