On 06/20/2017 06:51 AM, Mengxing Liu wrote:
> But in my benchmark, the throughput decrease by 15% after the modification.
> Can you help me do a quick review to find if there is anything wrong?
> I also attached the flame graph before/after the modification for reference.
Hmm. The hash table ought to speed up the RWConflictExists() function
right? Where in the flame graph is RWConflictExists()? If it only
accounts for a small amount of the overall runtime, even drastic speedup
there won't make much difference to the total.
> Here is my questions:
> 1. Is there any function in HTAB like “clear” that can make itself empty quickly?
> In this patch, when releasing a transaction object, I need to scan the hash table and
> use “hash_search” to remove entries one by one. It would make releasing operation slower.
Nope, there is no such function, I'm afraid.
> In a previous email, I reported that many backends wait for the lock “SerializableFinishedListLock”;
> If we don't implement functions like “ReleaseOneSerializableXact” quickly, they will be the bottleneck.
Yeah, that's probably what's causing the decrease in throughput you're
seeing.
You might need to also keep the linked lists, and use the hash table to
only look up particular items in the linked list faster.
I was surprised to see that you're creating a lot of smallish hash
tables, three for each PredXact entry. I would've expected all the
PredXact entries to share the same hash tables, i.e. have only three
hash tables in total. That would be more flexible in allocating
resources among entries. (It won't help with the quick-release, though)
> 2. Is the HTAB thread-safe? I would focus on removing some unnecessary locks if possible.
Nope, you need to hold a lock while searching/manipulating a HTAB hash
table. If the hash table gets big and you start to see lock contention,
you can partition it so that each operation only needs to lock the one
partition covering the search key.
- Heikki
