immudb icon indicating copy to clipboard operation
immudb copied to clipboard

chore(pkg/database): use atomic for waitingCount in instrumentedRWMutex

Open arturmelanchyk opened this issue 1 year ago • 4 comments

This change minimizes lock contention when working with instrumentedRWMutex According to the attached benchmark Lock \ Unlock operations as well as RLock \ RUnlock gain ~50% performance boost while State's performance remains on the same level

arturmelanchyk avatar Jan 28 '24 11:01 arturmelanchyk

@arturmelanchyk , please sign commits with GPG

tomekkolo avatar Mar 04 '24 15:03 tomekkolo

Just a kind reminder @tomekkolo @SimoneLazzaris , branch has been rebased onto latest master and all commits signed with GPG

arturmelanchyk avatar Mar 18 '24 09:03 arturmelanchyk

Just a kind reminder @tomekkolo @SimoneLazzaris , branch has been rebased onto latest master and all commits signed with GPG

Looked at it once more, why using 64 bit ? If you follow the path where it is used (DatabaseHealth), it is anyway cut to 32bits at the end. And generally, no need for such big counter here. Also not sure about real gain, this endpoint is not called so much.

tomekkolo avatar Mar 18 '24 10:03 tomekkolo

@tomekkolo

  1. As to the real gain, database heavily rely on this instrumentedRWMutex type, its used everywhere instead of regular sync.Mutex and sync.RWMutex https://github.com/codenotary/immudb/blob/c7265c67d1a9d52940822332da82e0d543a86547/pkg/database/database.go#L147-L153

see list of methods that use it:

Screenshot 2024-03-23 at 12 14 42

as a result, even tiny improvement in this type will lead to noticeable speed boost

  1. As to the size, instance of this type is only created once, so comparing 32bit to 64 bit we would only save 32bits once. Also take into consideration that in previous version this type was int which would anyway compile to 64 bits on most of the modern computers, so technically we do not add anything. Moreover, due to how golang aling structs in memory in this specific case it will anyway use 64bit, not 32, even if you change the type to 32bit

arturmelanchyk avatar Mar 23 '24 11:03 arturmelanchyk