Re: space reserved for WAL record does not match what was written: panic on windows - Mailing list pgsql-hackers

On Fri, Oct 11, 2013 at 1:14 AM, Noah Misch <noah@leadboat.com> wrote:
> On Thu, Oct 10, 2013 at 03:23:30PM +0200, Andres Freund wrote:
>> On 2013-10-10 08:59:47 -0400, Robert Haas wrote:
>> > On Tue, Oct 8, 2013 at 6:24 PM, Andres Freund <andres@2ndquadrant.com> wrote:
>> > > Do you have a better alternative? Making the computation unconditionally
>> > > 64bit will have a runtime overhead and adding a StaticAssert in the
>> > > existing macro doesn't work because we use it in array sizes where gcc
>> > > balks.
>> > > We could try using inline functions, but that's not going to be pretty
>> > > either.
>> > >
>> > > I don't really see that many further usecases that will align 64bit
>> > > values on 32bit platforms, so I think we're ok for now.
>> >
>> > I'd be inclined to make the computation unconditionally 64-bit.  I
>> > doubt the speed penalty is enough to worry about, and I think we're
>> > going to have more and more cases where optimizing for 32-bit
>> > platforms is just not the right decision.
>>
>> MAXALIGN is used in several of PG's hottest functions in many
>> scenarios. att_align_nominal is used in slot_deform_tuple,
>> heap_deform_tuple, nocachegetattr, etc. So I don't think that's viable
>> yet. At least not with much more benefit than this...
>
> Agreed.  Besides performance, aligning a wider-than-pointer value is an
> unusual need; authors should think thrice before doing that.  I might have
> even defined the MAXALIGN64 macro in xlog.c rather than a core header.
>
> Incidentally, why does MAXALIGN64 use unsigned math while MAXALIGN uses signed
> math?

Well, if this is the consensus, then I think the dynamic shared memory
patch may need some revision.  In that patch, I used uint64 to
represent the size of the dynamic shared memory segment, sort of on
the theory that we were going to use this to be allocating big chunks
of dynamic shared memory for stuff like parallel sort.  In follow-on
patches I'm currently developing to actually do stuff with dynamic
shared memory, this results in extensive use of MAXALIGN64, and it
really kind of looks like it wants the whole set of alignment macros,
not just that one.  So option one is to leave the dsm code alone and
add the rest of the macros.

But if we're bent on minimizing the use of 64-bit arithmetic on 32-bit
systems, then presumably I should instead go back and retrofit that
patch to use Size rather than uint64 to represent the size of a
segment.  But then I have two concerns:

1. Is there any guarantee that sizeof(intptr_t) >= sizeof(size_t)?
(Note that Size is just a typedef for size_t, in c.h)

2. If intptr_t is a signed type, as it appears to be, and size_t is an
unsigned type, as I believe it to be, then is it safe to use the
macros written for the signed type with a value of the unsigned type?
Off-hand I can't see a problem there, but I'm not certain I'm not
missing something.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company