Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2010-02-23 Thread Bruce Momjian
Tom Lane wrote: > Stefan Kaltenbrunner writes: > > Tom Lane wrote: > >> Nobody suggested dismissing it. The point was that it hasn't been > >> tested adequately to justify applying it now. > > > not sure what testing people want to get done though (there are a fair > > amount of results and pro

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2010-02-23 Thread Tom Lane
Stefan Kaltenbrunner writes: > Tom Lane wrote: >> Nobody suggested dismissing it. The point was that it hasn't been >> tested adequately to justify applying it now. > not sure what testing people want to get done though (there are a fair > amount of results and profiles in the thread)? Robert

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2010-02-23 Thread Stefan Kaltenbrunner
Tom Lane wrote: Stefan Kaltenbrunner writes: hmm I tend to disagree, this patch was specifically done to address a hotspot I noticed under a given workload and it helped a lot for that workload(like getting 6000qps more is pretty neat imho). While people might not use fixed width chars that of

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2010-02-23 Thread Tom Lane
Stefan Kaltenbrunner writes: > hmm I tend to disagree, this patch was specifically done to address a > hotspot I noticed under a given workload and it helped a lot for that > workload(like getting 6000qps more is pretty neat imho). > While people might not use fixed width chars that often(which

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2010-02-23 Thread Stefan Kaltenbrunner
Alvaro Herrera wrote: Robert Haas escribió: On Mon, Feb 22, 2010 at 9:35 PM, Bruce Momjian wrote: What ever happened to this patch? I think it's unclear that all of the best and worst cases have been sufficiently tested and that the results are satisfactory. We have everything from massive

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2010-02-23 Thread Alvaro Herrera
Robert Haas escribió: > On Mon, Feb 22, 2010 at 9:35 PM, Bruce Momjian wrote: > > What ever happened to this patch? > > I think it's unclear that all of the best and worst cases have been > sufficiently tested and that the results are satisfactory. We have > everything from massive performance

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2010-02-23 Thread Robert Haas
On Mon, Feb 22, 2010 at 9:35 PM, Bruce Momjian wrote: > Tom Lane wrote: >> Jeremy Kerr writes: >> > Stephen, >> >> If the updated function is always faster when the overall string is at >> >> least, say, 16 characters long, >> >> > But that's not the case - the cost of the function (and the speed

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2010-02-22 Thread Bruce Momjian
Tom Lane wrote: > Jeremy Kerr writes: > > Stephen, > >> If the updated function is always faster when the overall string is at > >> least, say, 16 characters long, > > > But that's not the case - the cost of the function (and the speedup from > > the previous version) depends on the number of sp

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-26 Thread Andrew Dunstan
On Fri, June 26, 2009 11:39 am, Tom Lane wrote: > to...@tuxteam.de writes: >> On Fri, Jun 26, 2009 at 05:03:11PM +0200, Dimitri Fontaine wrote: >>> It's becoming somewhat tricky, but maybe the test to do for the >>> optimisation to get used is n >= threshold && str[n-6] == 0x20, àla >>> Boyer/Moor

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-26 Thread Tom Lane
to...@tuxteam.de writes: > On Fri, Jun 26, 2009 at 05:03:11PM +0200, Dimitri Fontaine wrote: >> It's becoming somewhat tricky, but maybe the test to do for the >> optimisation to get used is n >= threshold && str[n-6] == 0x20, àla >> Boyer/Moore? > That's cute. What about comparing the last ali

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-26 Thread tomas
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Fri, Jun 26, 2009 at 05:03:11PM +0200, Dimitri Fontaine wrote: > Le 26 juin 09 à 14:47, Jeremy Kerr a écrit : >> For the new function to be faster, we need to know that there are more >> than 6 (on average, depending on alignment) trailing spaces. >

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-26 Thread Dimitri Fontaine
Le 26 juin 09 à 14:47, Jeremy Kerr a écrit : For the new function to be faster, we need to know that there are more than 6 (on average, depending on alignment) trailing spaces. It's becoming somewhat tricky, but maybe the test to do for the optimisation to get used is n >= threshold && str[n-

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-26 Thread Tom Lane
Jeremy Kerr writes: > Stephen, >> If the updated function is always faster when the overall string is at >> least, say, 16 characters long, > But that's not the case - the cost of the function (and the speedup from > the previous version) depends on the number of spaces that there are at > the

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-26 Thread Jeremy Kerr
Stephen, > If the updated function is always faster when the overall string is at > least, say, 16 characters long, But that's not the case - the cost of the function (and the speedup from the previous version) depends on the number of spaces that there are at the end. For the new function to

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-26 Thread Stephen Frost
* Jeremy Kerr (j...@ozlabs.org) wrote: > > Is it just the size that matters, or is it when there are few spaces > > at the end? > > It's the number of spaces at the end. If we knew this number, then we > wouldn't have to do any comparisons at all :) I meant in terms of affecting the performance

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-26 Thread Jeremy Kerr
Stephen, > Is it just the size that matters, or is it when there are few spaces > at the end? It's the number of spaces at the end. If we knew this number, then we wouldn't have to do any comparisons at all :) Cheers, Jeremy -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.o

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-26 Thread Stephen Frost
* Dimitri Fontaine (dfonta...@hi-media.com) wrote: > Le 26 juin 09 à 05:20, Jeremy Kerr a écrit : >>> Unfortunately, the cases with lots of padding spaces are probably >>> much less probable than the cases with fewer. It would be unpleasant >>> for example if this patch resulted in a severe perfor

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-26 Thread Dimitri Fontaine
Hi, Le 26 juin 09 à 05:20, Jeremy Kerr a écrit : Unfortunately, the cases with lots of padding spaces are probably much less probable than the cases with fewer. It would be unpleasant for example if this patch resulted in a severe performance degradation for a "canonical" example of char(n) bei

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-25 Thread Jeremy Kerr
Tom, > > I've put together some data from a microbenchmark of the bcTrulen > > function, patched and unpatched. > > Uh, where's the data? If you're after the raw data for a run, I've put it up: http://ozlabs.org/~jk/projects/db/data/bctruelen.csv I've also packaged up the quick-and-dirty bench

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-25 Thread Tom Lane
Jeremy Kerr writes: > I've put together some data from a microbenchmark of the bcTrulen > function, patched and unpatched. Uh, where's the data? > In the worst cases, I see a 53% cost increase on x86 (with the string > 'aaa ') and a 97% increase on PowerPC ('a '). > So, it all depends on the

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-25 Thread Jeremy Kerr
Hi Stephen, > What would be really useful would be "best case" and "worst case" > scenarios. I've put together some data from a microbenchmark of the bcTrulen function, patched and unpatched. As for best-case, if you have a long string of trailing spaces, we can go through them at theoreticall

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-24 Thread Stephen Frost
Stefan, * Stefan Kaltenbrunner (ste...@kaltenbrunner.cc) wrote: > Stephen Frost wrote: >> What would be really useful would be "best case" and "worst case" >> scenarios. Ideally, with profile information for this specific function >> (in addition to full benchmark runs since those can show minima

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-24 Thread Stefan Kaltenbrunner
Stephen Frost wrote: * Stefan Kaltenbrunner (ste...@kaltenbrunner.cc) wrote: FWIW: I'm able to measure an even more significant improvement of around 10%: What would be really useful would be "best case" and "worst case" scenarios. Ideally, with profile information for this specific function

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-24 Thread Stephen Frost
* Stefan Kaltenbrunner (ste...@kaltenbrunner.cc) wrote: > FWIW: I'm able to measure an even more significant improvement of around > 10%: What would be really useful would be "best case" and "worst case" scenarios. Ideally, with profile information for this specific function (in addition to ful

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-23 Thread Stefan Kaltenbrunner
Robert Haas wrote: On Tue, Jun 23, 2009 at 10:46 PM, Jeremy Kerr wrote: Robert, I'd still like to know the workload and exact numbers. From up-thread: http://ozlabs.org/~jk/projects/db/data/postgres.bcTruelen/ - or were there other details you were looking for? Oh! Very nice, sorry, mis

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-23 Thread Robert Haas
On Tue, Jun 23, 2009 at 10:46 PM, Jeremy Kerr wrote: > Robert, > >> I'd still like to know the workload and exact numbers. > > From up-thread: > >  http://ozlabs.org/~jk/projects/db/data/postgres.bcTruelen/ > > - or were there other details you were looking for? Oh! Very nice, sorry, missed the o

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-23 Thread Jeremy Kerr
Robert, > I'd still like to know the workload and exact numbers. From up-thread: http://ozlabs.org/~jk/projects/db/data/postgres.bcTruelen/ - or were there other details you were looking for? Cheers, Jeremy -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make chan

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-23 Thread Robert Haas
On Tue, Jun 23, 2009 at 9:05 PM, Jeremy Kerr wrote: > Results in a small performance increase; around 1-2% on my POWER6 test > box. I'd still like to know the workload and exact numbers. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subs

[HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-23 Thread Jeremy Kerr
The current bcTruelen function uses a simple reverse array scan to find the legth of a space-padded string. On some workloads (it shows in sysbench), this can result in a lot of time spent in this function. This change introduces a word-at-a-time comparison in bcTruelen, aiming to reduce the numbe

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Tom Lane
Greg Stark writes: > We could add an integer prefix to the char() datatype > with the "total" length and then just not include the spaces. But that > would make it not binary compatible with text -- which would mean > implementing a whole bunch of casts and operators. Um, it's already not binary

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Greg Stark
On Thu, Jun 18, 2009 at 10:03 PM, Simon Riggs wrote: > > The overall problem is that we expect the Datum's of a datatype to know > how to display themselves without any access to metadata. Yes > Another way of looking at this might be that we need a default FORMAT > specifier associated with a co

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Simon Riggs
On Thu, 2009-06-18 at 12:58 -0400, Tom Lane wrote: > "Kevin Grittner" writes: > > Tom Lane wrote: > >> It would be way nicer if we could strip trailing blanks on storage, > >> and then figure a way to either reconstitute them on output > > > How about pushing it even farther back -- always ke

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Kevin Grittner
Tom Lane wrote: > "Kevin Grittner" writes: >> Tom Lane wrote: >>> It would be way nicer if we could strip trailing blanks on >>> storage, and then figure a way to either reconstitute them on >>> output > >> How about pushing it even farther back -- always keep them with >> trimmed trailing s

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Tom Lane
"Kevin Grittner" writes: > Tom Lane wrote: >> It would be way nicer if we could strip trailing blanks on storage, >> and then figure a way to either reconstitute them on output > How about pushing it even farther back -- always keep them with > trimmed trailing spaces and add trailing spaces a

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Alvaro Herrera
Simon Riggs escribió: > I notice we lose on tuple access also. CHAR(n) is fixed length, but is > treated as variable length for offsets. Fixed character length != fixed byte length -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting,

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Simon Riggs
On Thu, 2009-06-18 at 09:59 -0400, Tom Lane wrote: > Simon Riggs writes: > > Why is bcTruelen being called so many *more* times? > > I think you have misunderstood the context. err, no, I just misread the original text. Possibly a worse error :-? > It would be way nicer if we could strip trail

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Kevin Grittner
Tom Lane wrote: > It would be way nicer if we could strip trailing blanks on storage, > and then figure a way to either reconstitute them on output How about pushing it even farther back -- always keep them with trimmed trailing spaces and add trailing spaces as required in operator functions

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Tom Lane
Simon Riggs writes: > Why is bcTruelen being called so many *more* times? I think you have misunderstood the context. The char(n) code is defined to store trailing blanks (up to n) but to disregard the trailing blanks during comparisons. bcTrueLen is invoked during comparisons (not during stora

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Florian Weimer
* Simon Riggs: > On Thu, 2009-06-18 at 15:09 +0200, Stefan Kaltenbrunner wrote: > >> the testcase discusses here is indeed CHAR(n) vs. VARCHAR. > > OK, thanks for pointing out my error. But I think your point still makes sense. Is it really necessary to determine the unpadded length for a query

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Simon Riggs
On Thu, 2009-06-18 at 15:09 +0200, Stefan Kaltenbrunner wrote: > the testcase discusses here is indeed CHAR(n) vs. VARCHAR. OK, thanks for pointing out my error. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Stefan Kaltenbrunner
Simon Riggs wrote: On Thu, 2009-06-18 at 15:14 +0300, Marko Kreen wrote: On 6/18/09, Simon Riggs wrote: On Tue, 2009-06-16 at 10:23 -0400, Tom Lane wrote: > Speaking of which, what about some performance numbers? Like Heikki, > I'm quite suspicious of whether there is any real-world gain t

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Simon Riggs
On Thu, 2009-06-18 at 15:14 +0300, Marko Kreen wrote: > On 6/18/09, Simon Riggs wrote: > > On Tue, 2009-06-16 at 10:23 -0400, Tom Lane wrote: > > > Speaking of which, what about some performance numbers? Like Heikki, > > > I'm quite suspicious of whether there is any real-world gain to be had

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Marko Kreen
On 6/18/09, Simon Riggs wrote: > On Tue, 2009-06-16 at 10:23 -0400, Tom Lane wrote: > > Speaking of which, what about some performance numbers? Like Heikki, > > I'm quite suspicious of whether there is any real-world gain to be had > > from this approach. > > > It has been "lore" for some tim

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Simon Riggs
On Tue, 2009-06-16 at 10:23 -0400, Tom Lane wrote: > Speaking of which, what about some performance numbers? Like Heikki, > I'm quite suspicious of whether there is any real-world gain to be had > from this approach. It has been "lore" for some time that VARCHAR is cheaper than VARCHAR(n), so I

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-18 Thread Simon Riggs
On Tue, 2009-06-16 at 00:24 -0400, Robert Haas wrote: > There's not much point taking the length of the word Not sure why we need to be calculating the length here anyway. ISTM that there is no need to reconfirm the length of the data, since it is already checked to be that length at insert. Wh

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-17 Thread Stefan Kaltenbrunner
Jeremy Kerr wrote: Hi all, Speaking of which, what about some performance numbers? OK, benchmarks done: http://ozlabs.org/~jk/projects/db/data/postgres.bcTruelen/ Summary: small increase in performance (~1-2% on my machine), at about 1.5 standard deviations from the mean. Profiles show a

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-17 Thread Gurjeet Singh
On Wed, Jun 17, 2009 at 3:26 PM, Jeremy Kerr wrote: > Hurjeet, > > > http://wiki.postgresql.org/wiki/Working_with_Git#Context_diffs_with_G > >it > > Awesome, thanks. > > I'm going to wait for a decision before reformatting and sending the > diff. > You might want to submit the patch (if modified

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-17 Thread Stefan Kaltenbrunner
Heikki Linnakangas wrote: Jeremy Kerr wrote: Speaking of which, what about some performance numbers? OK, benchmarks done: http://ozlabs.org/~jk/projects/db/data/postgres.bcTruelen/ Summary: small increase in performance (~1-2% on my machine), at about 1.5 standard deviations from the mean.

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-17 Thread Heikki Linnakangas
Jeremy Kerr wrote: Speaking of which, what about some performance numbers? OK, benchmarks done: http://ozlabs.org/~jk/projects/db/data/postgres.bcTruelen/ Summary: small increase in performance (~1-2% on my machine), at about 1.5 standard deviations from the mean. Profiles show a decent dro

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-17 Thread Gurjeet Singh
On Wed, Jun 17, 2009 at 3:01 PM, Jeremy Kerr wrote: > Will re-send the patch once I work out how to get git to create a > context diff... > http://wiki.postgresql.org/wiki/Working_with_Git#Context_diffs_with_Git -- Lets call it Postgres EnterpriseDB http://www.enterprisedb.com gurjeet[.

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-17 Thread Jeremy Kerr
Hi all, > Speaking of which, what about some performance numbers? OK, benchmarks done: http://ozlabs.org/~jk/projects/db/data/postgres.bcTruelen/ Summary: small increase in performance (~1-2% on my machine), at about 1.5 standard deviations from the mean. Profiles show a decent drop in hits

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Chuck McDevitt
runner; Gurjeet Singh > Subject: Re: [HACKERS] [PATCH] backend: compare word-at-a-time in > bcTruelen > On 64-bit machines, the native word size is 64-bits (obviously), and comparing 32 bits at a time is much slower than comparing 64 bits at a time. You might want to consider this. -- Sent

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Stefan Kaltenbrunner
Jeremy Kerr wrote: Hi Tom, Speaking of which, what about some performance numbers? Like Heikki, I'm quite suspicious of whether there is any real-world gain to be had from this approach. Will send numbers tomorrow, with the reworked patch. I can easily redo my testing as well if required.

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Jeremy Kerr
Hi Tom, > Speaking of which, what about some performance numbers? Like Heikki, > I'm quite suspicious of whether there is any real-world gain to be > had from this approach. Will send numbers tomorrow, with the reworked patch. Cheers, Jeremy -- Sent via pgsql-hackers mailing list (pgsql-hac

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Tom Lane
Greg Stark writes: > On some architectures like intel accessing unaligned ints is just > slow. On others (Alpha and PPC iirc?) it is an immediate bus error. To a first approximation, Intel is the *only* popular architecture that doesn't bus-error on unaligned accesses. (And I'm sure their chip d

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Jeremy Kerr
Hi all, > That's correct. To check the alignment you would have to look at the > actual pointer. I would suggest using the existing macros to handle > alignment. Hm, though the only one I see offhand which is relevant is > the moderately silly PointerIsAligned(). Still it would make the code > cle

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Andrew Dunstan
Robert Haas wrote: Ooh, good point. I still don't like the 0x20 thing, but using uint32 instead of int or long is the main point, unless we support any platforms where 0x20 != ' '. All our server encodings are strictly ASCII supersets. So 0x20 is always the space character. cheers a

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Robert Haas
On Tue, Jun 16, 2009 at 8:38 AM, Greg Stark wrote: > On Tue, Jun 16, 2009 at 1:03 PM, Robert Haas wrote: >> I see that... but I don't think the test in the first loop is correct. >>  It's based on the value of i % 4, but I'm not convinced that you know >> anything about the alignment at the point w

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Greg Stark
On Tue, Jun 16, 2009 at 1:41 PM, Stephen Frost wrote: > > Ah, you may be half right there (see below).  It does appear to be > assuming that char *s (or s[i == 0]) is aligned, which isn't a > guarentee (in fact, it might never be right..).  If having it actually > aligned is an important bit (as op

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Stephen Frost
* Greg Stark (gsst...@mit.edu) wrote: > There are two points here that kind of cancel each other out :) Thanks for the insight. :) Stephen signature.asc Description: Digital signature

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Greg Stark
On Tue, Jun 16, 2009 at 1:03 PM, Robert Haas wrote: > >> On the flip side, I am curious as to if the arguments to a stored >> procedure are always aligned or not.  Never had a case to care before, >> but if palloc() is always going to return an aligned chunk of memory >> (per MemSetAligned in c.h)

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: > I see that... but I don't think the test in the first loop is correct. > It's based on the value of i % 4, but I'm not convinced that you know > anything about the alignment at the point where i == 0. Ah, you may be half right there (see below). It

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Greg Stark
On Tue, Jun 16, 2009 at 1:03 PM, Robert Haas wrote: > I see that... but I don't think the test in the first loop is correct. >  It's based on the value of i % 4, but I'm not convinced that you know > anything about the alignment at the point where i == 0. That's correct. To check the alignment you

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Robert Haas
On Tue, Jun 16, 2009 at 6:30 AM, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: >> As I look at this, another problem is that it seems to me that you're >> assuming that VARDATA_ANY() will return an aligned pointer, which >> isn't necessarily the case (see src/include/postgres.

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-16 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: > As I look at this, another problem is that it seems to me that you're > assuming that VARDATA_ANY() will return an aligned pointer, which > isn't necessarily the case (see src/include/postgres.h). I believe you need to look at it more carefully. I do

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-15 Thread Heikki Linnakangas
Robert Haas wrote: The advice in Stephen's email is also very good - in particular, whatever you come up with, you should submit performance results. Note that while --enable-profiling is very useful and profiling numbers are good to submit, you'll also want to make sure you do a build that is op

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-15 Thread Robert Haas
On Mon, Jun 15, 2009 at 9:51 PM, Jeremy Kerr wrote: > I was considering something like: > >        unsigned int spaces; >        const unsigned int wordsize = sizeof(unsigned int); > >        memset(&spaces, ' ', wordsize); > > In most cases, the compiler should be able to optimise the memset out,

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-15 Thread Stephen Frost
Jeremy, * Jeremy Kerr (j...@ozlabs.org) wrote: > Signed-off-by: Jeremy Kerr > > --- > src/backend/utils/adt/varchar.c | 24 +--- > 1 file changed, 21 insertions(+), 3 deletions(-) Thanks for the contribution. A couple of comments: The documentation for submitting a patc

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-15 Thread Jeremy Kerr
Robert, > This looks very non-portable to me. Unsurprisingly, I'm new to postgres hacking and the large number of supported platforms :) I was considering something like: unsigned int spaces; const unsigned int wordsize = sizeof(unsigned int); memset(&spaces, ' ', word

Re: [HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-15 Thread Robert Haas
On Jun 15, 2009, at 9:04 PM, Jeremy Kerr wrote: Signed-off-by: Jeremy Kerr --- src/backend/utils/adt/varchar.c | 24 +--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/ varchar.c index 5f3c658..688

[HACKERS] [PATCH] backend: compare word-at-a-time in bcTruelen

2009-06-15 Thread Jeremy Kerr
Signed-off-by: Jeremy Kerr --- src/backend/utils/adt/varchar.c | 24 +--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/varchar.c index 5f3c658..6889dff 100644 --- a/src/backend/utils/adt/varchar.c +++