FAQ

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.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Search Discussions

  • Andres Freund at Jun 27, 2012 at 4:10 pm

    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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedJun 27, '12 at 3:10p
activeJun 27, '12 at 4:10p
posts2
users2
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2021 Grokbase