Use dashmap in a couple of low hanging places #112
Conversation
| guard.queries.shrink_to_fit(); | ||
| guard.stats.hits = 0; | ||
| guard.stats.misses = 0; | ||
| // This is the only place we acquire the write lock, in order to synchronize clearing the cache |
There was a problem hiding this comment.
Why do we need an RwLock at all? Both stats and queries are synchronized, we can just clear the DashMap and reset the counters to zero. DashMap has its own RwLock, so we're double-locking for each access to the cache.
There was a problem hiding this comment.
probably just out of an abundance of caution, making this operation atomic seemed nice but I guess its just fine if the stats are a few cycles behind the map.
|
Thank you for the PR. I'm going to benchmark it to see if there is a significant difference between the two approaches. The query parser gets hit only in multi-shard configuration and when the client is using prepared statements (or the extended protocol)*. The peers/discovery module isn't used that heavily (yet). I wrote it before realizing most public clouds disable multicast on their network. There is another place where DashMap could be used: pgdog/pgdog/src/frontend/comms.rs Line 34 in 53612de This place is actually pretty hot: it's touched on every transaction/query and every time a client connects/disconnects. *Correction: and when the config contains a primary and replicas. It's used for read/write traffic separation. |
| /// Number of connected clients. | ||
| pub fn clients_len(&self) -> usize { | ||
| self.global.clients.lock().len() |
There was a problem hiding this comment.
Seems like this function and len served the same purpose?
IDK if that's what you had in mind in #85. I think another two easy followups are:
SipHashto something faster but less DOS resistant like foldhash.