Re: [GENERAL] Is float8 a reference type?

[Paul A Jungwirth]

On Sat, Sep 23, 2017 at 9:40 AM, Tom Lane  wrote:
> I wonder whether you're using up-to-date Postgres headers (ones
> where Float8GetDatum is a static inline function).

I'm building against 9.6.3 on both machines. I'm not doing anything
special to change the compilation options. Here is my whole Makefile:

MODULES = floatfile
EXTENSION = floatfile
EXTENSION_VERSION = 1.0.0
DATA = floatfile--$(EXTENSION_VERSION).sql

PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)

But what I'm really interested in is this: What are the bad things
that can happen if I do `datums = (Datum *)floats`, as long as it's
only when Datums are 8 bytes wide? Is there a platform with
pass-by-val float8s where that won't work?

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

Re: [GENERAL] Is float8 a reference type?

[Tom Lane]

Paul A Jungwirth  writes:
> 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).

I wonder whether you're using up-to-date Postgres headers (ones
where Float8GetDatum is a static inline function).  For me, both
of those platforms recognize it as a no-op --- in fact, clang
turns a loop like

for (i = 0; i < n; i++) {
datums[i] = Float8GetDatum(floats[i]);
}

into something that looks suspiciously like an inlined, loop-unrolled
memcpy().

regards, tom lane


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

Re: [GENERAL] Is float8 a reference type?

[Paul A Jungwirth]

On Fri, Sep 22, 2017 at 8:38 PM, Tom Lane  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:

  .loc2 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)`):

  .loc2 393 21 is_stmt 1  ## floatfile.c:393:21
  movslq%r15d, %r13
  .loc2 393 28 is_stmt 0  ## floatfile.c:393:28
  leaq(,%r13,8), %rdi
  .loc2 393 14## floatfile.c:393:14
  callq_palloc
  movq%rax, %r12
Ltmp177:
  ##DEBUG_VALUE: load_floatfile:i <- 0
  .loc2 394 19 is_stmt 1 discriminator 1 ## floatfile.c:394:19
  testl%r15d, %r15d
Ltmp178:
  .loc2 394 5 is_stmt 0 discriminator 1 ## floatfile.c:394:5
  jeLBB7_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
  .loc2 0 5 discriminator 1   ## floatfile.c:0:5
  movq-72(%rbp), %rbx
Ltmp180:
  ##DEBUG_VALUE: load_floatfile:floats <- %RBX
  xorl%r14d, %r14d
Ltmp181:
  .p2align4, 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]
  .loc2 395 34 is_stmt 1  ## floatfile.c:395:34
  movsd(%rbx,%r14,8), %xmm0## xmm0 = mem[0],zero
  .loc2 395 19 is_stmt 0  ## floatfile.c:395:19
  callq_Float8GetDatum
  .loc2 395 17## floatfile.c:395:17
  movq%rax, (%r12,%r14,8)
Ltmp182:
  .loc2 394 30 is_stmt 1 discriminator 2 ## floatfile.c:394:30
  incq%r14
  .loc2 394 19 is_stmt 0 discriminator 1 ## floatfile.c:394:19
  cmpq%r13, %r14
Ltmp183:
  .loc2 394 5 discriminator 1 ## floatfile.c:394:5
  jlLBB7_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

Re: [GENERAL] Is float8 a reference type?

[Tom Lane]

Paul A Jungwirth  writes:
> On Fri, Sep 22, 2017 at 8:05 PM, Pavel Stehule  
> wrote:
>> I don't think so it is good idea to write 64bit only extensions.

> I agree, but how about this?:

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

regards, tom lane


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

Re: [GENERAL] Is float8 a reference type?

[Pavel Stehule]

2017-09-23 5:10 GMT+02:00 Paul A Jungwirth :

> On Fri, Sep 22, 2017 at 8:05 PM, Pavel Stehule 
> wrote:
> > yes, it is 8 bytes on 64-bit.
>
> Thanks!
>
> > I don't think so it is good idea to write 64bit only extensions.
>
> I agree, but how about this?:
>
> if (FLOAT8PASSBYVAL) {
>   datums = (Datum *)floats;
> } else {
>   datums = palloc0(arrlen * sizeof(Datum));
>   for (i = 0; i < arrlen; i++) {
> datums[i] = Float8GetDatum(floats[i]);
>   }
> }
>

it can work.

You have to solve deallocation in only one path. palloc0 is not necessary
in this case.


>
> Thanks,
> Paul
>

Re: [GENERAL] Is float8 a reference type?

[Paul A Jungwirth]

On Fri, Sep 22, 2017 at 8:05 PM, Pavel Stehule  wrote:
> yes, it is 8 bytes on 64-bit.

Thanks!

> I don't think so it is good idea to write 64bit only extensions.

I agree, but how about this?:

if (FLOAT8PASSBYVAL) {
  datums = (Datum *)floats;
} else {
  datums = palloc0(arrlen * sizeof(Datum));
  for (i = 0; i < arrlen; i++) {
datums[i] = Float8GetDatum(floats[i]);
  }
}

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

Re: [GENERAL] Is float8 a reference type?

[Pavel Stehule]

2017-09-23 4:52 GMT+02:00 Paul A Jungwirth :

> The docs say that a Datum can be 4 bytes or 8 depending on the machine:
>
> https://www.postgresql.org/docs/9.5/static/sql-createtype.html
>
> Is a Datum always 8 bytes for 64-bit architectures?
>
> And if so, can my C extension skip a loop like this when compiling
> there, and just do a memcpy (or even a cast)?:
>

yes, it is 8 bytes on 64-bit.

I don't think so it is good idea to write 64bit only extensions.



> float8 *floats;
> Datum *datums;
>
> datums = palloc(arrlen * sizeof(Datum));
> for (i = 0; i < arrlen; i++) {
>   datums[i] = Float8GetDatum(floats[i]);
> }
>
> 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
>

Re: [GENERAL] Is float8 a reference type?

[Paul A Jungwirth]

On Fri, Sep 22, 2017 at 7:52 PM, Paul A Jungwirth
 wrote:
> Is a Datum always 8 bytes for 64-bit architectures?

Never mind, I found this in `pg_config.h`:

/* float8, int8, and related values are passed by value if 'true', by
   reference if 'false' */
#define FLOAT8PASSBYVAL true

Paul


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