Hi, On 2017-09-20 18:26:50 +0530, amul sul wrote: > Patch 0007:
Thanks for looking! > 1: > 400 + /* > 401 + * XXX: might be worthwhile to only handle oid sysattr, to > reduce > 402 + * overhead - it's the most common key. > 403 + */ > > IMHO, let fix that as well. I tested this by fixing (see the attach patch) > but does > not found much gain on my local centos vm (of course, the pgbench load > wasn't big enough). I ended up with a bigger patch, that removes all extractions from tuples, by storing the extracted column in an array. > 2: How about have wrapping following condition in SearchCatCacheMiss() by > unlikely(): > > if (IsBootstrapProcessingMode()) > return NULL; Given this is the cache miss case, I can't get excited about it - there's several 100ks of cycles to access the heap via an indexscan... > 3: Can we have following assert in SearchCatCacheN() instead > SearchSysCacheN(), so that we'll assert direct SearchCatCacheN() call > as well? > > Assert(SysCache[cacheId]->cc_nkeys == <N>); Done, although I kept the others too. > Other than these concern, patch looks pretty reasonable to me. I'd appreciate if you could have a look at the new version as well. Regards, Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
