codenotary / immudb

immudb - immutable database based on zero trust, SQL/Key-Value/Document model, tamperproof, data change history
https://immudb.io
Other
8.54k stars 341 forks source link

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

Open arturmelanchyk opened 6 months ago

arturmelanchyk commented 6 months ago

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

tomekkolo commented 5 months ago

@arturmelanchyk , please sign commits with GPG

arturmelanchyk commented 5 months ago

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

tomekkolo commented 5 months ago

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.

arturmelanchyk commented 5 months ago

@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