On Fri, Sep 22, 2017 at 8:38 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> "Premature optimization is the root of all evil".  Do you have good reason
> to think that it's worth your time to write unsafe/unportable code?  Do
> you know that your compiler doesn't turn Float8GetDatum into a no-op
> already?  (Mine does, on a 64-bit machine.)

Ha ha, thank you for keeping me honest! But can you explain what is
unsafe about the cast? For a little more context: I've loaded a float8
array from a file, but I need to pass a Datum array to
construct_md_array. With an 8-byte Datum, I can just pass the original
float array, right? But with smaller Datums I need to go through the
array and convert each element. (I'm not really worried about these
files being moved between machines, so I'm willing to make the on-disk
format the same as the in-memory format.)

Since I'm expecting ~10 million elements per array, it seems like
skipping the conversion will have a real effect. I checked the
assembly and do see a difference (on both Mac+clang and Linux+gcc).
Here is the Mac command line:

    platter:floatfile paul$ clang -Wall -Wmissing-prototypes
-Wpointer-arith -Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -Wno-unused-command-line-argument -O2  -I. -I./
-I/usr/local/Cellar/postgresql@9.6/9.6.3/include/server
-I/usr/local/Cellar/postgresql@9.6/9.6.3/include/internal
-I/usr/local/opt/gettext/include -I/usr/local/opt/openldap/include
-I/usr/local/opt/openssl/include -I/usr/local/opt/readline/include
-I/usr/local/opt/tcl-tk/include -g -S -o floatfile.s floatfile.c

Here is the assembly for the cast:

      .loc    2 391 23 is_stmt 1      ## floatfile.c:391:23
      movq    -48(%rbp), %r15
    Ltmp176:
      ##DEBUG_VALUE: load_floatfile:datums <- %R15

Here is the assembly for the loop (after just changing the code to `if
(FLOAT8PASSBYVAL && false)`):

      .loc    2 393 21 is_stmt 1      ## floatfile.c:393:21
      movslq    %r15d, %r13
      .loc    2 393 28 is_stmt 0      ## floatfile.c:393:28
      leaq    (,%r13,8), %rdi
      .loc    2 393 14                ## floatfile.c:393:14
      callq    _palloc
      movq    %rax, %r12
    Ltmp177:
      ##DEBUG_VALUE: load_floatfile:i <- 0
      .loc    2 394 19 is_stmt 1 discriminator 1 ## floatfile.c:394:19
      testl    %r15d, %r15d
    Ltmp178:
      .loc    2 394 5 is_stmt 0 discriminator 1 ## floatfile.c:394:5
      je    LBB7_11
    Ltmp179:
    ## BB#9:
      ##DEBUG_VALUE: load_floatfile:arrlen <- %R15D
      ##DEBUG_VALUE: load_floatfile:nulls <- [%RBP+-80]
      ##DEBUG_VALUE: load_floatfile:floats <- [%RBP+-72]
      ##DEBUG_VALUE: load_floatfile:filename <- %RBX
      .loc    2 0 5 discriminator 1   ## floatfile.c:0:5
      movq    -72(%rbp), %rbx
    Ltmp180:
      ##DEBUG_VALUE: load_floatfile:floats <- %RBX
      xorl    %r14d, %r14d
    Ltmp181:
      .p2align    4, 0x90
    LBB7_10:                                ## =>This Inner Loop Header: Depth=1
      ##DEBUG_VALUE: load_floatfile:floats <- %RBX
      ##DEBUG_VALUE: load_floatfile:arrlen <- %R15D
      ##DEBUG_VALUE: load_floatfile:nulls <- [%RBP+-80]
      .loc    2 395 34 is_stmt 1      ## floatfile.c:395:34
      movsd    (%rbx,%r14,8), %xmm0    ## xmm0 = mem[0],zero
      .loc    2 395 19 is_stmt 0      ## floatfile.c:395:19
      callq    _Float8GetDatum
      .loc    2 395 17                ## floatfile.c:395:17
      movq    %rax, (%r12,%r14,8)
    Ltmp182:
      .loc    2 394 30 is_stmt 1 discriminator 2 ## floatfile.c:394:30
      incq    %r14
      .loc    2 394 19 is_stmt 0 discriminator 1 ## floatfile.c:394:19
      cmpq    %r13, %r14
    Ltmp183:
      .loc    2 394 5 discriminator 1 ## floatfile.c:394:5
      jl    LBB7_10
    Ltmp184:
    LBB7_11:
      ##DEBUG_VALUE: load_floatfile:arrlen <- %R15D
      ##DEBUG_VALUE: load_floatfile:nulls <- [%RBP+-80]

I get the same results on gcc too: the palloc, the loop, and even
`call Float8GetDatum@PLT`.

I'll do some timing of each version too, but it doesn't look like a
pointless optimization. I'd still like to know what is unsafe about it
though.

Thanks!
Paul


-- 
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

Reply via email to