On Wed, Nov 24, 2010 at 11:33 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, Nov 24, 2010 at 10:25 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Or make it execute only in assert-enabled mode, perhaps.
>
>> But making the check execute only in assert-enabled more
>> doesn't seem right, since the check actually acts to mask other coding
>> errors, rather than reveal them. Maybe we replace the check with one
>> that only occurs in an Assert-enabled build and just loops through and
>> does Assert(PrivateRefCount[i] == 0).
>
> Yeah, that would be sensible. There is precedent for this elsewhere
> too; I think there's a similar setup for checking buffer refcounts
> during transaction cleanup.
>
>> I'm not sure exactly where this
>> gets called in the shutdown sequence, though - is it sensible to
>> Assert() here?
>
> Assert is sensible anywhere.
OK, patch attached. Here's what oprofile output looks like with this applied:
3505 10.4396 libc-2.11.2.so memset
2051 6.1089 libc-2.11.2.so memcpy
1686 5.0217 postgres AllocSetAlloc
1642 4.8907 postgres hash_search_with_hash_value
1247 3.7142 libc-2.11.2.so _int_malloc
1096 3.2644 libc-2.11.2.so fread
855 2.5466 ld-2.11.2.so do_lookup_x
723 2.1535 ld-2.11.2.so _dl_fixup
645 1.9211 ld-2.11.2.so strcmp
620 1.8467 postgres MemoryContextAllocZero
Somehow I don't think I'm going to get much further with this without
figuring out how to get oprofile to cough up a call graph.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company