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

Read-Write Locks #735

Merged
merged 10 commits into from
Jul 22, 2023
Merged

Read-Write Locks #735

merged 10 commits into from
Jul 22, 2023

Conversation

vyzo
Copy link
Collaborator

@vyzo vyzo commented Jul 21, 2023

Sorely missing them! Cherry picked from #728

@vyzo vyzo requested review from fare, ober and drewc July 21, 2023 21:36
Copy link
Collaborator

@fare fare left a comment

Choose a reason for hiding this comment

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

I didn't read in detail, looks generally good, but

  1. Using a FFI-based solution might be better, especially if it can take advantage of futexes on Linux yet be portable to other systems... sigh.
  2. There is no documentation.

@vyzo
Copy link
Collaborator Author

vyzo commented Jul 22, 2023

FFI is no go, we need ro play nice with the gambit scheduler.
Documentation i shall add.

@vyzo
Copy link
Collaborator Author

vyzo commented Jul 22, 2023

added documentation.

@vyzo vyzo added this to the Gerbil18 milestone Jul 22, 2023
doc/reference/misc.md Outdated Show resolved Hide resolved
the word preempt is just wrong.
@vyzo vyzo requested a review from fare July 22, 2023 09:27
Copy link
Collaborator

@fare fare left a comment

Choose a reason for hiding this comment

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

Maybe a short comment that this form of "unlock" relocks is in order. I admit I am not familiar enough with this primitive.

@vyzo
Copy link
Collaborator Author

vyzo commented Jul 22, 2023

Actually I was wrong and you are right that we need to relock at the top of the loop: http://dynamo.iro.umontreal.ca/~gambit/wiki/index.php?title=Documentation:Procedure_mutex-unlock!

vyzo added 4 commits July 22, 2023 13:52
reading the docs every now and then is good; bug averted.
Use two separate convars, one for readers and one for writers.
Readers wait on the rcv, while writes wait on the wcv.
When the read lock is relinquished, if there are writers waiting we
wake up exactly one with signal.
When the write lock is relinquished, if there are writers waiting we
wake up exactly one with signal; otherwise we wake up all readers.
fingers crossed that CI won't completely mess up the timings.
it can help with debugging.
@fare
Copy link
Collaborator

fare commented Jul 22, 2023

Having to re-lock and retry suggests that you should maintain an explicit queue rather than busy poll, and then use a condition variable to wake up the waiting writer or readers.

@vyzo
Copy link
Collaborator Author

vyzo commented Jul 22, 2023

there is no busy poll, and the two condvars eliminate the contention.

@vyzo vyzo merged commit 2f6650c into master Jul 22, 2023
@vyzo vyzo deleted the rwlock branch July 22, 2023 13:29
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