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

Proper mutexing #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Proper mutexing #1

wants to merge 1 commit into from

Conversation

s00500
Copy link
Contributor

@s00500 s00500 commented Aug 5, 2022

Hi Kasper,

First of all, I know I mentioned it often but PLEASE try to use earlie return, it is a standard pattern and makes code much faster to reason about, also it shows where functions end without scrolling to the end of them. Keep in mind that code is never only read by oneself, and even oneself benefits massively from having easy to read code in a couple of weeks time
Secondly: Your .Iter function gets you in major trouble, as it passes a complex object and then marshals it for as many times as there are clients on individual go routines. That is not only inefficient, but also causes more potential datarace headaches than needed. I have changed it to a global broadcast function that does the marshaling once and then passes the bytes to the individual ws connections. Bytes are straight forward and safe to copy, and the expensive marshal function is only called once, in a syncronous fashion, meaning it is protected by the mutex that should still be RLocked in the function that calls Broadcast. Also it is easier to read than the function passing + channel operation construct

Finally we can remove the very expensive Copier calls and library. Copier uses reflections inside and should really only ever be used as a very last resort. (or even never, as it could potentially make things worse like it did here)
Also then the Mutex on the individual entries can be removed, as in every situation one would need to lock the parent mutex on .entries anyways. Removing this spares you from several dozends of locks and unlocks that are redundant.

As a final point: using RWMutexes instead of the normal ones can give some advantages (there can be multiple RLocks at the same time but only ever one Lock used for writing)

Hope this helps, let me know if you have more questions

Remove zeroconfig entry mutexes
Make sure zeroconfig entries are locked on sub go routines
Earlie returns
Handly more errors

Signed-off-by: Lukas Bachschwell <[email protected]>
@s00500 s00500 requested a review from kasperskaarhoj August 5, 2022 13:43
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.

1 participant