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

On Thu, Oct 17, 2013 at 08:39:56AM -0400, Robert Haas wrote:
> 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:
> >> > 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:

I'm not bent on _minimizing_ use of 64-bit arithmetic on 32-bit systems, but I
disfavor an addition of such usage rippling through various hot paths of the
system indiscriminately.  Making a design choice to use *ALIGN64 in a
particular module doesn't bother me that way.

> 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)

C99 doesn't require it, but I have never heard of a platform where it is
false.  sizeof(intptr_t) > sizeof(size_t) systems have existed.

> 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.

Yes; we do that all the time, e.g. the MAXALIGN call in AllocSetAlloc().
Looping back to my question above, I think it doesn't matter (on a two's
complement system) whether the macro uses a signed type or an unsigned type.
It changes the type of the result; that's all.  Nonetheless, we should be
consistent about signedness between the regular and 64-bit macro variants.

Thanks,
nm

-- 
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com