Hi,
> git branch also available at:
> http://git.postgresql.org/gitweb/?p=users/rhaas/postgres.git;a=shortlog;h=refs/heads/chash2014
A minor review of this:
* Should be rebased ontop of the atomics API
* In benchmarks it becomes apparent that the dynamic element width makes some macros like CHashTableGetNode() and
CHashTableGetGarbageByBucket()quite expensive. At least gcc on x86 can't figure how to build an efficient expression
forthe target. There's two ways to address this: a) Actually morph chash into something that will be included into the
user sites. Since I think there'll only be a limited number of those that's probably acceptable. b) Simplify the
accessrules. I quite seriously doubt that the interleaving of garbage and freelists is actually necessary/helpful.
* There's several cases where it's noticeable that chash creates more cacheline misses - that's imo a good part of why
thesingle threaded performance regresses. There's good reasons for the current design without a inline bucket, namely
thatit makes the concurrency handling easier. But I think that can be countered by still storing a pointer - just to an
elementinside the bucket. Afaics that'd allow this to be introduced quite easily?
I'm afraid that I can't see us going forward unless we address the noticeable single threaded penalties. Do you see
thatdifferently?
* There's some whitespace damage. (space followed by tab, followed by actual contents)
* I still think it's a good idea to move the full memory barriers into the cleanup/writing process by doing write
memorybarriers when acquiring a hazard pointer and RMW atomic operations (e.g. atomic add) in the process testing for
thehazard pointer.
* Shouldn't we relax the CPU in a couple places like CHashAllocate(), CHashBucketScan()'s loops?
* I don't understand right now (but then I'm in a Hotel bar) why it's safe for CHashAddToGarbage() to clobber the hash
value?CHashBucketScan() relies the chain being ordered by hashcode. No? Another backend might just have been past the
IsMarked()bit and look at the hashcode?
* We really should find a way to sensibly print the stats.
Greetings,
Andres Freund
