Re: Reduce the memcpy call from SearchCatCache

Hi,
Here is the oprofile results of pgbench.

CPU: P4 / Xeon with 2 hyper-threads, speed 2793.55 MHz (estimated)
Counted GLOBAL_POWER_EVENTS events
with a unit mask of 0x01 (mandatory) count 100000
samples % app name symbol name
134521 6.8312 ipmi_si (no symbols)
94515 4.7996 vmlinux schedule
52609 2.6716 postgres AllocSetAlloc
39659 2.0140 postgres base_yyparse
34605 1.7573 vmlinux mwait_idle
33234 1.6877 vmlinux _spin_lock
31353 1.5922 libc-2.3.4.so memcpy

I think that the performance improves if the call frequency of memcpy
is reduced. I measured the place where postgres used memcpy.
(Test program is pgbench -t 4000)

total-size avg-size caller
------------------------------------------------------------------------
636185 111968560 176 catcache.c:1129
68236 18436197 270 xlog.c:947
3909 13822874 3536 xlog.c:940
20003 3520528 176 catcache.c:1376
56010 2071477 36 pgstat.c:2288
125524 1902864 15 dynahash.c:948
20001 1760088 88 setrefs.c:205

catcache.c:1129 is memcpy at SearchCatCache, and catcache.c:1376
is memcpy at SearchCatCacheList.

memcpy(cur_skey, cache->cc_skey, sizeof(cur_skey));

Attached patch is reduce the memcpy calls from SearchCatCache
and SearchCatCacheList. This patch directly uses cache->cc_skey
in looking for hash table.
Here is an effect of the patch.

original: Counted GLOBAL_POWER_EVENTS events
samples % app name symbol name
31353 1.5922 libc-2.3.4.so memcpy

patched: Counted GLOBAL_POWER_EVENTS events
samples % app name symbol name
20629 1.0684 libc-2.3.4.so memcpy

---
Atsushi Ogawa

Atsushi Ogawa <a_ogawa(at)hi-ho(dot)ne(dot)jp> writes:
> Attached patch is reduce the memcpy calls from SearchCatCache
> and SearchCatCacheList. This patch directly uses cache->cc_skey
> in looking for hash table.

How much did you test this patch? I'm fairly sure it will break things.
There are cases where cache lookups happen recursively.

regards, tom lane

Tom Lane writes:
> Atsushi Ogawa <a_ogawa(at)hi-ho(dot)ne(dot)jp> writes:
> > Attached patch is reduce the memcpy calls from SearchCatCache
> > and SearchCatCacheList. This patch directly uses cache->cc_skey
> > in looking for hash table.
>
> How much did you test this patch? I'm fairly sure it will break
> things.
> There are cases where cache lookups happen recursively.

I tested regression test and pgbench. However, I did not consider
recursive case. I revised a patch for safe recursive call.
But I cannot find test case in which recursive call happens.

In my understanding, recursive call at SearchCatCache does not happen
while looking for hash table. The recursive call happens while reading
the relation. If the cache->cc_skey is copied before read the relation,
I think it is safe.

best regards,

--- Atsushi Ogawa

Atsushi Ogawa <a_ogawa(at)hi-ho(dot)ne(dot)jp> writes:
> Tom Lane writes:
>> There are cases where cache lookups happen recursively.

> I tested regression test and pgbench. However, I did not consider
> recursive case. I revised a patch for safe recursive call.
> But I cannot find test case in which recursive call happens.

Try turning on CLOBBER_CACHE_ALWAYS or CLOBBER_CACHE_RECURSIVELY to
get a demonstration of what can happen under the right conditions.

I think the only really safe way to do what you propose would be to
refactor the ScanKey API to separate the datum values and is-null
flags from the more static parts of the data structure. That would
be a pretty large/invasive patch, and the numbers cited here don't
seem to me to justify the work. It's even possible that it could
end up being a net performance loss due to having to pass around more
pointers :-(

regards, tom lane