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
lib/model: Use a single lock #9275
Conversation
I grow weary of the fmut/pmut shenanigans. This consolidates both under one lock; I'm not convinced there are any significant performance differences with this approach since we're literally just protecting map juggling... - The locking goes away when we were already under an appropriate fmut lock. - Where we had fmut.RLock()+pmut.Lock() it gets upgraded to an fmut.Lock(). - Otherwise s/pmut/fmut/. In order to avoid diff noise for an important change I did not do the following cleanups, which will be filed in a PR after this one, if accepted: - Renaming fmut to just mut - Renaming methods that refer to being "PRLocked" and stuff like that. - Removing the no longer relevant deadlock detector.
* main: lib/nat: Fix test build failure (ref syncthing#9010)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have done this ages ago, on the other hand, I think we claim things are now stable'ish?
Like a canoe, it is perfectly stable for the competent user... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We claim things are stable-ish now, as in it's time to start meddle with model locks again?! :D
Cleanup after #9275. This renames `fmut` -> `mut`, removes the deadlock detector and associated plumbing, renames some things from `...PRLocked` to `...RLocked` and similar, and updates comments. Apart from the removal of the deadlock detection machinery, no functional code changes... i.e. almost 100% diff noise, have fun reviewing.
* main: (89 commits) build: Update quic-go (fixes syncthing#9287) lib/model: Only handle relevant folder summaries (kqueue) (fixes syncthing#9183) (syncthing#9288) lib/model: Use a single lock (phase two: cleanup) (syncthing#9276) build(deps): bump actions/setup-go from 4 to 5 (syncthing#9279) lib/model: Use a single lock (syncthing#9275) cmd/syncthing: Better cli stdin handling (ref syncthing#9166) (syncthing#9281) cmd/syncthing: Mostly replace urfave/cli command line parser with alecthomas/kong (syncthing#9166) lib/nat: Fix test build failure (ref syncthing#9010) lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274) lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274) lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271) lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271) lib/nat, lib/upnp: IPv6 UPnP support (syncthing#9010) gui, man, authors: Update docs, translations, and contributors gui: Show folder/device status on small screens (syncthing#8643) lib/model: Remove runner during folder cleanup (fixes syncthing#9269) (syncthing#9271) build: Update dependencies (syncthing#9265) build: Revert specifics for Go 1.21.4, build using Go 1.21.5 (syncthing#9264) lib/fs: Reduce memory usage in xattrs handling (syncthing#9251) lib/model: Improve LastSeen handling (syncthing#9256) ...
* main: lib/model: Use a single lock (phase two: cleanup) (syncthing#9276) build(deps): bump actions/setup-go from 4 to 5 (syncthing#9279) lib/model: Use a single lock (syncthing#9275) cmd/syncthing: Better cli stdin handling (ref syncthing#9166) (syncthing#9281) cmd/syncthing: Mostly replace urfave/cli command line parser with alecthomas/kong (syncthing#9166) lib/nat: Fix test build failure (ref syncthing#9010) lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274) lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274) lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271) lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271) lib/nat, lib/upnp: IPv6 UPnP support (syncthing#9010) gui, man, authors: Update docs, translations, and contributors gui: Show folder/device status on small screens (syncthing#8643) lib/model: Remove runner during folder cleanup (fixes syncthing#9269) (syncthing#9271)
I'm tired of the fmut/pmut shenanigans. This consolidates both under one lock; I'm not convinced there are any significant performance differences with this approach since we're literally just protecting map juggling...
In order to avoid diff noise for an important change I did not do the following cleanups, which will be filed in a PR after this one, if accepted: