Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-30 Thread Simon Riggs
On 30 April 2013 06:57, Simon Riggs si...@2ndquadrant.com wrote: I'm about to light up the build farm with a trial commit of the compiler instructions stuff. Amazingly that seemed to work. ISTM that we also need this patch to put memory barriers in place otherwise the code might be

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-30 Thread Andres Freund
On 2013-04-30 11:55:29 +0100, Simon Riggs wrote: ISTM that we also need this patch to put memory barriers in place otherwise the code might be rearranged. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services ---

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-30 Thread Ants Aasma
On Tue, Apr 30, 2013 at 1:55 PM, Simon Riggs si...@2ndquadrant.com wrote: On 30 April 2013 06:57, Simon Riggs si...@2ndquadrant.com wrote: I'm about to light up the build farm with a trial commit of the compiler instructions stuff. Amazingly that seemed to work. Thanks for committing. Sorry

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-30 Thread Simon Riggs
On 30 April 2013 12:23, Ants Aasma a...@cybertec.at wrote: ISTM that we also need this patch to put memory barriers in place otherwise the code might be rearranged. The compiler and CPU both have to preserve correctness when rearranging code, so I don't think we care about it here. It might

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-30 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes: ISTM that we also need this patch to put memory barriers in place otherwise the code might be rearranged. This is simply silly. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-30 Thread Simon Riggs
On 30 April 2013 15:51, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: ISTM that we also need this patch to put memory barriers in place otherwise the code might be rearranged. This is simply silly. You crack me up sometimes. Yes, it is; seem to be having a bad

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-30 Thread Greg Smith
I re-ran the benchmark that's had me most worried against the committed code and things look good so far. I've been keeping quiet because my tests recently have all agreed with what Ants already described. This is more a confirmation summary than new data. The problem case has been Jeff's

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-30 Thread Martijn van Oosterhout
On Tue, Apr 30, 2013 at 01:05:30PM -0400, Greg Smith wrote: I re-ran the benchmark that's had me most worried against the committed code and things look good so far. I've been keeping quiet because my tests recently have all agreed with what Ants already described. This is more a

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-30 Thread Greg Smith
On 4/30/13 5:26 PM, Martijn van Oosterhout wrote: I came across this today: Data Integrity Extensions, basically a standard for have an application calculate a checksum of a block and submitting it together with the block so that the disk can verify that the block it is writing matches what the

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-30 Thread Noah Misch
Orthogonal to this thread, but: On Tue, Apr 30, 2013 at 06:39:09PM -0400, Greg Smith wrote: The WAL logging of hint bits is where the scary stuff to me for this feature has always been at. My gut feel is that doing that needed to start being available as an option anyway. Just this month

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-30 Thread Andres Freund
On 2013-04-30 18:39:09 -0400, Greg Smith wrote: The WAL logging of hint bits is where the scary stuff to me for this feature has always been at. My gut feel is that doing that needed to start being available as an option anyway. Just this month we've had two customer issues pop up where we

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-29 Thread Simon Riggs
On 28 April 2013 13:22, Ants Aasma a...@cybertec.at wrote: I have updated the base patch. This is supposed to go under the cflags-vector patch that Jeff posted yesterday. I've committed your patch, with two changes * one comment extended * adding the .h file from Jeff's last main patch

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-28 Thread Ants Aasma
On Fri, Apr 26, 2013 at 10:57 PM, Jeff Davis pg...@j-davis.com wrote: On Fri, Apr 26, 2013 at 7:09 AM, Simon Riggs si...@2ndquadrant.com wrote: I'm expecting to spend some time on this over the weekend, once I've re-read the thread and patches to see if there is something to commit. That's my

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-26 Thread Ants Aasma
On Apr 25, 2013 10:38 PM, Jeff Davis pg...@j-davis.com wrote: On Tue, 2013-04-23 at 11:44 +0300, Ants Aasma wrote: I will try to reword. Did you have a chance to clarify this, as well as some of the other documentation issues Simon mentioned here?

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-26 Thread Florian Pflug
On Apr26, 2013, at 10:28 , Ants Aasma ants.aa...@eesti.ee wrote: On Apr 25, 2013 10:38 PM, Jeff Davis pg...@j-davis.com wrote: On Tue, 2013-04-23 at 11:44 +0300, Ants Aasma wrote: I will try to reword. Did you have a chance to clarify this, as well as some of the other documentation

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-26 Thread Andres Freund
On 2013-04-26 13:11:00 +0200, Florian Pflug wrote: The unresolved code issue that I know of is moving the compiler flags behind a configure check. I would greatly appreciate it if you could take a look at that. My config-fu is weak and it would take me some time to figure out how to do

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-26 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: On 2013-04-26 13:11:00 +0200, Florian Pflug wrote: The unresolved code issue that I know of is moving the compiler flags behind a configure check. I would greatly appreciate it if you could take a look at that. My config-fu is weak and it would

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-26 Thread Simon Riggs
On 26 April 2013 14:40, Tom Lane t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com writes: On 2013-04-26 13:11:00 +0200, Florian Pflug wrote: The unresolved code issue that I know of is moving the compiler flags behind a configure check. I would greatly appreciate it if you could

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-26 Thread Jeff Davis
On Fri, Apr 26, 2013 at 7:09 AM, Simon Riggs si...@2ndquadrant.com wrote: I'm expecting to spend some time on this over the weekend, once I've re-read the thread and patches to see if there is something to commit. That's my last time window, so this looks like the last chance to make changes

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-26 Thread Greg Smith
On 4/26/13 3:57 PM, Jeff Davis wrote: The second patch adds the configure-time check for the right compilation flags, and uses them when compiling checksum.c. I called the new variable CFLAGS_EXTRA, for lack of a better idea, so feel free to come up with a new name. It doesn't check for, or use,

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-26 Thread Jeff Davis
On Fri, 2013-04-26 at 16:40 -0400, Greg Smith wrote: I think I need to do two baselines: master without checksums, and master with extra optimizations but still without checksums. It may be the case that using better compile time optimizations gives a general speedup that's worth

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-26 Thread Andres Freund
On 2013-04-26 12:57:09 -0700, Jeff Davis wrote: I updated the patch and split it into two parts (attached). The second patch adds the configure-time check for the right compilation flags, and uses them when compiling checksum.c. I called the new variable CFLAGS_EXTRA, for lack of a better

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-26 Thread Jeff Davis
On Sat, 2013-04-27 at 00:20 +0200, Andres Freund wrote: CFLAGS_VECTORIZATION? EXTRA sounds to generic to me. I went with CFLAGS_VECTOR to be a little shorter while still keeping some meaning. I think it would be better to have a PGAC_PROG_CC_VAR_OPT or so which assigns the flag to some passed

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-25 Thread Jeff Davis
On Tue, 2013-04-23 at 11:44 +0300, Ants Aasma wrote: I will try to reword. Did you have a chance to clarify this, as well as some of the other documentation issues Simon mentioned here? http://www.postgresql.org/message-id/CA+U5nMKVEu8UDXQe +Nk=d7nqm4ypfszaef0esak4j31lyqc...@mail.gmail.com I'm

[HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-23 Thread Jeff Davis
Starting a new thread to more narrowly address the topic. Attached is my reorganization of Ants's patch here: http://www.postgresql.org/message-id/CA +CSw_vinyf-w45i=M1m__MpJZY=e8s4nt_knnpebtwjtoa...@mail.gmail.com My changes: * wrest the core FNV algorithm from the specific concerns of a data

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-23 Thread Florian Pflug
On Apr23, 2013, at 09:17 , Jeff Davis pg...@j-davis.com wrote: I'd lean toward simplicity and closer adherence to the published version of the algorithm rather than detecting a few more obscure error patterns. It looks like the modification slows down the algorithm, too. The pattern that plain

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-23 Thread Ants Aasma
On Apr 23, 2013 10:17 AM, Jeff Davis pg...@j-davis.com wrote: Attached is my reorganization of Ants's patch here: http://www.postgresql.org/message-id/CA +CSw_vinyf-w45i=M1m__MpJZY=e8s4nt_knnpebtwjtoa...@mail.gmail.com Thanks for your help. Some notes below. My changes: * wrest the core

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-23 Thread Andres Freund
On 2013-04-23 00:17:28 -0700, Jeff Davis wrote: + # important optimization flags for checksum.c + ifeq ($(GCC),yes) + checksum.o: CFLAGS += -msse4.1 -funroll-loops -ftree-vectorize + endif I am pretty sure we can't do those unconditionally: - -funroll-loops and -ftree-vectorize weren't always

Re: [HACKERS] Substituting Checksum Algorithm (was: Enabling Checksums)

2013-04-23 Thread Ants Aasma
On Tue, Apr 23, 2013 at 11:47 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-04-23 00:17:28 -0700, Jeff Davis wrote: + # important optimization flags for checksum.c + ifeq ($(GCC),yes) + checksum.o: CFLAGS += -msse4.1 -funroll-loops -ftree-vectorize + endif I am pretty sure we