On Wednesday, June 27, 2012 05:09:46 PM Heikki Linnakangas wrote:
> On 27.06.2012 17:53, Andres Freund wrote:
> > I had noticed one thing when reviewing the patches before:
> >
> > @@ -717,6 +719,15 @@ XLogInsert(RmgrId rmid, uint8 info, XLogRecData
> > *rdata)
> >
> > bool doPageWrites;
> > bool isLogSwitch = (rmid == RM_XLOG_ID&& info ==
> > XLOG_SWITCH); uint8 info_orig = info;
> >
> > + static XLogRecord *rechdr;
> > +
> > + if (rechdr == NULL)
> > + {
> > + rechdr = malloc(SizeOfXLogRecord);
> > + if (rechdr == NULL)
> > + elog(ERROR, "out of memory");
> > + MemSet(rechdr, 0, SizeOfXLogRecord);
> > + }
> >
> > /* cross-check on whether we should be here or not */
> > if (!XLogInsertAllowed())
> >
> > Why do you allocate this dynamically? XLogRecord is 32bytes, there
> > doesn't seem to be much point in this?
>
> On 64-bit architectures, the struct needs padding at the end to make the
> size MAXALIGNed to 32 bytes; a simple "XLogRecord rechdr" local variable
> would not include that. You could do something like:
>
> union
> {
> XLogRecord rechdr;
> char bytes[SizeOfXLogRecord];
> }
>
> but that's quite ugly too.
Ah, yes. Makes sense.
Btw, the XLogRecord struct is directly layed out with 32bytes here (64bit,
linux) because xl_prev needs to be 8byte aligned...
Andres
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services