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

session#domainMap use RWMutex replace Mutex #33737

Closed
likzn opened this issue Apr 6, 2022 · 1 comment · Fixed by #33739
Closed

session#domainMap use RWMutex replace Mutex #33737

likzn opened this issue Apr 6, 2022 · 1 comment · Fixed by #33739
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@likzn
Copy link
Contributor

likzn commented Apr 6, 2022

Enhancement

The Get() method of domainMap reading more and write less , so using mutex for synchronization is a serious waste of resources, which can be optimized from the following two aspects

  1. Use RWMutex to replace Mutex synchronization block: it can improve the performance of reading and avoid blocking between reading and reading
  2. Narrow the range of locks: Before using Defer to synchronize a large block of statements, we can separate locks, synchronize when reading, and synchronize when writing to reduce blocking time

The core change is insession/tidb.go as below as first edtion

index 911e64f37..236759b15 100644
--- a/session/tidb.go
+++ b/session/tidb.go
@@ -44,13 +44,11 @@ import (
 
 type domainMap struct {
        domains map[string]*domain.Domain
-       mu      sync.Mutex
+       mu      sync.RWMutex
 }
 
 func (dm *domainMap) Get(store kv.Storage) (d *domain.Domain, err error) {
-       dm.mu.Lock()
-       defer dm.mu.Unlock()
-
+       dm.mu.RLock()
        // If this is the only domain instance, and the caller doesn't provide store.
        if len(dm.domains) == 1 && store == nil {
                for _, r := range dm.domains {
@@ -60,6 +58,8 @@ func (dm *domainMap) Get(store kv.Storage) (d *domain.Domain, err error) {
 
        key := store.UUID()
        d = dm.domains[key]
+       dm.mu.RUnlock()
+
        if d != nil {
                return
        }
@@ -92,8 +92,7 @@ func (dm *domainMap) Get(store kv.Storage) (d *domain.Domain, err error) {
        if err != nil {
                return nil, err
        }
-       dm.domains[key] = d
-
+       dm.Set(store, d)
        return
 }
 
@@ -103,6 +102,12 @@ func (dm *domainMap) Delete(store kv.Storage) {
        dm.mu.Unlock()
 }
 
+func (dm *domainMap) Set(store kv.Storage, domain *domain.Domain) {
+       dm.mu.Lock()
+       dm.domains[store.UUID()] = domain
+       dm.mu.Unlock()
+}
+
 var (
        domap = &domainMap{
                domains: map[string]*domain.Domain{},
(
@likzn
Copy link
Contributor Author

likzn commented Apr 6, 2022

@tiancaiamao @xhebox @XuHuaiyu PTAL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
1 participant