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.



Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to