Re: space reserved for WAL record does not match what was written: panic on windows

On Fri, Oct 18, 2013 at 1:39 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Fri, Oct 11, 2013 at 1:14 AM, Noah Misch <noah(at)leadboat(dot)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(at)2ndquadrant(dot)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.
>
>
For me I don't really see why there's a need to use MAXALIGN64 for any
memory addresses related to RAM.
I only created MAXALIGN64 because I needed it to fix the WAL code which
needed as 64bit type on all platforms, not just 64bit ones. For me it made
perfect sense, so I'm a bit confused at most of this fuss. Though I do
understand that it's a bit weird that both macros are almost the same on a
64 bit machine...

As for signed vs unsigned, I've not looked at all of the places where
MAXALIGN is used, but I just assumed it was for memory addresses, if this
is the case then I'm confused why we'd ever want a negative valued memory
address?

This might be an obvious one, but can anyone tell me why the casts are in
the macro at all? Can a compiler not decide for itself which type it should
be using?

Regards

David Rowley

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