Thread: char() overhead on read-only workloads not so insignifcant as the docs claim it is...

char() overhead on read-only workloads not so insignifcant as the docs claim it is...

From
Stefan Kaltenbrunner
Date:
I'm currently doing some benchmarking on a Nehalem 
box(http://www.kaltenbrunner.cc/blog/index.php?/archives/26-Benchmarking-8.4-Chapter-1Read-Only-workloads.html) 
with 8.4 and while investigating what looks like issues in pgbench I 
also noticed that using char() has more than a negligable overhead on 
some (very special) readonly(!) workloads.

for example running sysbench in read-only mode against 8.4 results in a 
profile(for the full run) that looks similiar to:

samples  %        symbol name
981690   11.0656  bcTruelen
359183    4.0487  index_getnext
311128    3.5070  AllocSetAlloc
272330    3.0697  hash_search_with_hash_value
258157    2.9099  LWLockAcquire
195673    2.2056  _bt_compare
190303    2.1451  slot_deform_tuple
168101    1.8948  PostgresMain
164191    1.8508  _bt_checkkeys
126110    1.4215  FunctionCall2
123965    1.3973  SearchCatCache
120629    1.3597  LWLockRelease

the default sysbench mode actually uses a number of different queries 
and the ones dealing with char() are actually only a small part of the 
full set of queries sent.
The specific query is causing bcTruelen to show up in the profile is:

"SELECT c from sbtest where id between $1 and $2 order by c" where the 
parameters are for example
$1 = '5009559', $2 = '5009658' - ie ranges of 100.


benchmarking only that query results in:

samples  %        symbol name
2148182  23.5861  bcTruelen
369463    4.0565  index_getnext
362784    3.9832  AllocSetAlloc
284198    3.1204  slot_deform_tuple
185279    2.0343  _bt_checkkeys
180119    1.9776  LWLockAcquire
172733    1.8965  appendBinaryStringInfo
144158    1.5828  internal_putbytes
141040    1.5486  AllocSetFree
138093    1.5162  printtup
124255    1.3643  hash_search_with_hash_value
117054    1.2852  heap_form_minimal_tuple

at around 46000 queries/s

changing the fault sysbench schema from:
                             Table "public.sbtest" Column |      Type      |                      Modifiers 

--------+----------------+----------------------------------------------------- id     | integer        | not null
default
 
nextval('sbtest_id_seq'::regclass) k      | integer        | not null default 0 c      | character(120) | not null
default''::bpchar pad    | character(60)  | not null default ''::bpchar
 
Indexes:    "sbtest_pkey" PRIMARY KEY, btree (id)    "k" btree (k)


to                              Table "public.sbtest" Column |       Type        |                      Modifiers 

--------+-------------------+----------------------------------------------------- id     | integer           | not
nulldefault 
 
nextval('sbtest_id_seq'::regclass) k      | integer           | not null default 0 c      | character varying | not
nulldefault ''::character varying pad    | character(60)     | not null default ''::bpchar
 
Indexes:    "sbtest_pkey" PRIMARY KEY, btree (id)    "k" btree (k)

results in a near 50%(!) speedup in terms of tps to around 67000 
queries/s. This is however an extreme case because the c column actually 
contains no data at all (except for an empty string).

the profile for the changed testcase looks like:
430797    5.2222  index_getnext
396750    4.8095  AllocSetAlloc
345508    4.1883  slot_deform_tuple
228222    2.7666  appendBinaryStringInfo
227766    2.7610  _bt_checkkeys
193818    2.3495  LWLockAcquire
179925    2.1811  internal_putbytes
168871    2.0471  printtup
152026    1.8429  AllocSetFree
146333    1.7739  heap_form_minimal_tuple
144305    1.7493  FunctionCall2
128320    1.5555  hash_search_with_hash_value


at the very least we should reconsider this part of our docs:

"  There is no performance difference between these three types, apart 
from increased storage space when using the blank-padded type, and a few 
extra CPU cycles to check the length when storing into a 
length-constrained column."

from http://www.postgresql.org/docs/8.4/static/datatype-character.html



regards

Stefan


<div dir="ltr">Comments?<br /><br /><div class="gmail_quote">On Sat, Jun 13, 2009 at 3:44 PM, Stefan Kaltenbrunner
<spandir="ltr"><stefan@kaltenbrunner.cc></span> wrote:<br /><blockquote class="gmail_quote" style="border-left:
1pxsolid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> I'm currently doing some benchmarking on a
Nehalembox(<a
href="http://www.kaltenbrunner.cc/blog/index.php?/archives/26-Benchmarking-8.4-Chapter-1Read-Only-workloads.html"
target="_blank">http://www.kaltenbrunner.cc/blog/index.php?/archives/26-Benchmarking-8.4-Chapter-1Read-Only-workloads.html</a>)
with8.4 and while investigating what looks like issues in pgbench I also noticed that using char() has more than a
negligableoverhead on some (very special) readonly(!) workloads.<br /><br /> for example running sysbench in read-only
modeagainst 8.4 results in a profile(for the full run) that looks similiar to:<br /><br /> samples  %        symbol
name<br/> 981690   11.0656  bcTruelen<br /> 359183    4.0487  index_getnext<br /> 311128    3.5070  AllocSetAlloc<br />
272330   3.0697  hash_search_with_hash_value<br /> 258157    2.9099  LWLockAcquire<br /> 195673    2.2056
 _bt_compare<br/> 190303    2.1451  slot_deform_tuple<br /> 168101    1.8948  PostgresMain<br /> 164191    1.8508
 _bt_checkkeys<br/> 126110    1.4215  FunctionCall2<br /> 123965    1.3973  SearchCatCache<br /> 120629    1.3597
 LWLockRelease<br/><br /> the default sysbench mode actually uses a number of different queries and the ones dealing
withchar() are actually only a small part of the full set of queries sent.<br /> The specific query is causing
bcTruelento show up in the profile is:<br /><br /> "SELECT c from sbtest where id between $1 and $2 order by c" where
theparameters are for example<br /> $1 = '5009559', $2 = '5009658' - ie ranges of 100.<br /><br /><br /> benchmarking
onlythat query results in:<br /><br /> samples  %        symbol name<br /> 2148182  23.5861  bcTruelen<br /> 369463  
 4.0565 index_getnext<br /> 362784    3.9832  AllocSetAlloc<br /> 284198    3.1204  slot_deform_tuple<br /> 185279  
 2.0343 _bt_checkkeys<br /> 180119    1.9776  LWLockAcquire<br /> 172733    1.8965  appendBinaryStringInfo<br /> 144158
  1.5828  internal_putbytes<br /> 141040    1.5486  AllocSetFree<br /> 138093    1.5162  printtup<br /> 124255  
 1.3643 hash_search_with_hash_value<br /> 117054    1.2852  heap_form_minimal_tuple<br /><br /> at around 46000
queries/s<br/><br /> changing the fault sysbench schema from:<br /><br />                             Table
"public.sbtest"<br/>  Column |      Type      |                      Modifiers <br />
--------+----------------+-----------------------------------------------------<br/>  id     | integer        | not
nulldefault nextval('sbtest_id_seq'::regclass)<br />  k      | integer        | not null default 0<br />  c      |
character(120)| not null default ''::bpchar<br />  pad    | character(60)  | not null default ''::bpchar<br />
Indexes:<br/>    "sbtest_pkey" PRIMARY KEY, btree (id)<br />    "k" btree (k)<br /><br /><br /> to<br />              
              Table "public.sbtest"<br />  Column |       Type        |                      Modifiers <br />
--------+-------------------+-----------------------------------------------------<br/>  id     | integer           |
notnull default nextval('sbtest_id_seq'::regclass)<br />  k      | integer           | not null default 0<br />  c    
 |character varying | not null default ''::character varying<br />  pad    | character(60)     | not null default
''::bpchar<br/> Indexes:<br />    "sbtest_pkey" PRIMARY KEY, btree (id)<br />    "k" btree (k)<br /><br /> results in a
near50%(!) speedup in terms of tps to around 67000 queries/s. This is however an extreme case because the c column
actuallycontains no data at all (except for an empty string).<br /><br /> the profile for the changed testcase looks
like:<br/> 430797    5.2222  index_getnext<br /> 396750    4.8095  AllocSetAlloc<br /> 345508    4.1883
 slot_deform_tuple<br/> 228222    2.7666  appendBinaryStringInfo<br /> 227766    2.7610  _bt_checkkeys<br /> 193818  
 2.3495 LWLockAcquire<br /> 179925    2.1811  internal_putbytes<br /> 168871    2.0471  printtup<br /> 152026    1.8429
 AllocSetFree<br/> 146333    1.7739  heap_form_minimal_tuple<br /> 144305    1.7493  FunctionCall2<br /> 128320  
 1.5555 hash_search_with_hash_value<br /><br /><br /> at the very least we should reconsider this part of our docs:<br
/><br/> "  There is no performance difference between these three types, apart from increased storage space when using
theblank-padded type, and a few extra CPU cycles to check the length when storing into a length-constrained column."<br
/><br/> from <a href="http://www.postgresql.org/docs/8.4/static/datatype-character.html"
target="_blank">http://www.postgresql.org/docs/8.4/static/datatype-character.html</a><br/><br /><br /><br /> regards<br
/><br/> Stefan<br /><font color="#888888"><br /> -- <br /> Sent via pgsql-hackers mailing list (<a
href="mailto:pgsql-hackers@postgresql.org"target="_blank">pgsql-hackers@postgresql.org</a>)<br /> To make changes to
yoursubscription:<br /><a href="http://www.postgresql.org/mailpref/pgsql-hackers"
target="_blank">http://www.postgresql.org/mailpref/pgsql-hackers</a><br/></font></blockquote></div><br /><br
clear="all"/><br />-- <br />Lets call it Postgres<br /><br />EnterpriseDB      <a href="http://www.enterprisedb.com"
target="_blank">http://www.enterprisedb.com</a><br/><br />gurjeet[.singh]@EnterpriseDB.com<br /> singh.gurjeet@{ gmail
|hotmail | indiatimes | yahoo }.com<br /> Mail sent from my BlackLaptop device<br /></div> 
> On Sat, Jun 13, 2009 at 3:44 PM, Stefan Kaltenbrunner
> <stefan@kaltenbrunner.cc> wrote:

> > The specific query is causing bcTruelen to show up in the profile is:
> >
> > "SELECT c from sbtest where id between $1 and $2 order by c" where the
> > parameters are for example
> > $1 = '5009559', $2 = '5009658' - ie ranges of 100.
> >
> >
> > benchmarking only that query results in:
> >
> > samples  %        symbol name
> > 2148182  23.5861  bcTruelen
> > 369463    4.0565  index_getnext
> > 362784    3.9832  AllocSetAlloc

Gurjeet Singh escribió:
> Comments?

Maybe bcTruelen could be optimized to step on one word at a time
(perhaps by using XOR against a precomputed word filled with ' '),
instead of one byte at a time ...

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


Alvaro,

> Maybe bcTruelen could be optimized to step on one word at a time
> (perhaps by using XOR against a precomputed word filled with ' '),
> instead of one byte at a time ...

I have a patch for this, will send soon.

Regards,


Jeremy


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

From
Jeremy Kerr
Date:
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

---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
+++ b/src/backend/utils/adt/varchar.c
@@ -624,16 +624,34 @@ varchartypmodout(PG_FUNCTION_ARGS)static intbcTruelen(BpChar *arg){
+    const unsigned int spaces = 0x20202020;
+    const int    wordsize = sizeof(spaces);    char       *s = VARDATA_ANY(arg);    int            i;
-    int            len;
-    len = VARSIZE_ANY_EXHDR(arg);
-    for (i = len - 1; i >= 0; i--)
+    i = VARSIZE_ANY_EXHDR(arg) - 1;
+
+    /* compare down to an aligned boundary */
+    for (; i >= 0 && i % wordsize != wordsize - 1; i--)    {        if (s[i] != ' ')
+            return i + 1;
+    }
+
+    /* now that we're aligned, compare word at a time */
+    for (; i >= wordsize - 1; i -= wordsize)
+    {
+        if (*(unsigned int *)(s + i - (wordsize - 1)) != spaces)            break;    }
+
+    /* check within the last non-matching word */
+    for (; i >= 0; i--)
+    {
+        if (s[i] != ' ')
+            break;
+    }
+    return i + 1;}


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

From
Robert Haas
Date:
On Jun 15, 2009, at 9:04 PM, Jeremy Kerr <jk@ozlabs.org> wrote:

> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
>
> ---
> 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
> +++ b/src/backend/utils/adt/varchar.c
> @@ -624,16 +624,34 @@ varchartypmodout(PG_FUNCTION_ARGS)
> static int
> bcTruelen(BpChar *arg)
> {
> +    const unsigned int spaces = 0x20202020;
> +    const int    wordsize = sizeof(spaces);

This looks very non-portable to me.

...Robert


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

From
Jeremy Kerr
Date:
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, ' ', wordsize);

In most cases, the compiler should be able to optimise the memset out, 
but it may introduce overhead where this is not possible.

However, are there any supported platforms where sizeof(unsigned int) != 
4 ?

Cheers,


Jeremy


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

From
Stephen Frost
Date:
Jeremy,

* Jeremy Kerr (jk@ozlabs.org) wrote:
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
>
> ---
>  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 patch to PostgreSQL is here:
http://wiki.postgresql.org/wiki/Submitting_a_Patch

There is also a Developer FAQ available here:
http://wiki.postgresql.org/wiki/Developer_FAQ

The PostgreSQL core folks prefer context diffs (it's not my preference,
but I'm not part of core, nor am I a committer :).

There are a number of things requested to be included with a patch, but
in particular I would point out:

----
Which CVS branch the patch is against (ordinarily this will be HEAD).

Whether it compiles and tests successfully, so we know nothing obvious
is broken.

Whether it contains any platform-specific items and if so, has it been
tested on other platforms.

Describe the effect your patch has on performance, if any. If the patch
is intended to improve performance, it's a good idea to include some
reproducible tests to demonstrate the improvement.
----

You might check out sections 3 & 6 of src/include/c.h.  Section 3
defines standard system types, while section 6 defines some widely
useful macros; in particular our custom MemSet and MemSetAligned, which
work on aligned memory structures for improved performance.
Thanks,
    Stephen

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

From
Robert Haas
Date:
On Mon, Jun 15, 2009 at 9:51 PM, Jeremy Kerr<jk@ozlabs.org> 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,
> but it may introduce overhead where this is not possible.

What about just:

static char spaces[4] = { ' ', ' ', ' ', ' ' };

and then ... * (uint32 *) spaces?

There's not much point taking the length of the word when you've
initialized it to contain exactly 4 bytes.  What you want to do is
pick the flavor of integer that will be the same length as what you've
initialized, and it turns out we already have that (see
src/include/c.h).

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

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 optimized for speed (i.e. no profiling, no casserts, no
debug) and do timings on that.

...Robert


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

From
Heikki Linnakangas
Date:
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 optimized for speed (i.e. no profiling, no casserts, no
> debug) and do timings on that.

I'm particularly interested to see how much the patch hurts performance 
in the cases where it's not helping.

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


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

From
Stephen Frost
Date:
* Robert Haas (robertmhaas@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 don't think it's
making any such assumption.  Specifically, it has three loops; an "until
we're aligned" loop, then a "while we're aligned", and a "when we've
done all the aligned we could do".

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) it makes me wonder.
Thanks,
    Stephen

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

From
Robert Haas
Date:
On Tue, Jun 16, 2009 at 6:30 AM, Stephen Frost<sfrost@snowman.net> wrote:
> * Robert Haas (robertmhaas@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 don't think it's
> making any such assumption.  Specifically, it has three loops; an "until
> we're aligned" loop, then a "while we're aligned", and a "when we've
> done all the aligned we could do".

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
convincedthat you know 
anything about the alignment at the point where i == 0.

I might be all wet here, I haven't looked at this area of the code in detail.

> 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) it makes me wonder.

Well, if it's char(n) for n <~ 126, it's going to have a 1-byte
varlena header...

...Robert


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

From
Greg Stark
Date:
On Tue, Jun 16, 2009 at 1:03 PM, Robert Haas<robertmhaas@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.

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
clearer even if it's pretty simple.

Incidentally, the char foo[4] = {' ',' ',' ',' '} suggestion is, I
think, bogus. There would be no alignment guarantee on that array.
Personally I'm find with 0x20202020 with a comment explaining what it
is.







--
greg
http://mit.edu/~gsstark/resume.pdf


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

From
Stephen Frost
Date:
* Robert Haas (robertmhaas@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 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 opposed to just doing the comparisons in
larger but possibly unaligned blocks) then that'd make a difference.

If the code as-is showed performance improvment even when it's working
on less-than-aligned blocks, I'd be curious what would happen if it was
actually aligned.  Unfortunately, the results of such would probably be
heavily architecture dependent..

> > 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) it makes me wonder.
>
> Well, if it's char(n) for n <~ 126, it's going to have a 1-byte
> varlena header...

Right, but I'm talking about the base of the argument itself, not
the start of the data.  If every variable length argument to a stored
procedure is palloc()'d independently, and palloc()'s always return
aligned memory, we'd at least know that the base of the argument is
aligned and could figure out the header size and then do the
comparisons accordingly.  This would also mean, of course, that we'd
almost(?) never have s[i == 0] on an aligned boundary due to the
header.
Thanks,
    Stephen

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

From
Greg Stark
Date:
On Tue, Jun 16, 2009 at 1:03 PM, Robert Haas<robertmhaas@gmail.com> 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) it makes me wonder.
>
> Well, if it's char(n) for n <~ 126, it's going to have a 1-byte
> varlena header...

There are two points here that kind of cancel each other out :)

If the data is in fact returned from a palloc because it was the
result of some other function call then it will almost certainly have
a 4-byte header and that'll be aligned. There are some exceptions
where functions are just returning copies and copy the whole datum
though, but the point is we normally don't toast or pack varlenas
unless they're being stored on disk.

However that's all irrelevant because there's no guarantee the data
being passed will have been palloced at all. You could get a pointer
to data in a shared buffer. Ie, data on disk. That will be aligned
based on how tuples are packed on disk which is precisely where we go
out of our way to avoid wasting space on alignment.

--
greg
http://mit.edu/~gsstark/resume.pdf


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

From
Stephen Frost
Date:
* Greg Stark (gsstark@mit.edu) wrote:
> There are two points here that kind of cancel each other out :)

Thanks for the insight. :)
Stephen

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

From
Greg Stark
Date:
On Tue, Jun 16, 2009 at 1:41 PM, Stephen Frost<sfrost@snowman.net> 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 opposed to just doing the comparisons in
> larger but possibly unaligned blocks) then that'd make a difference.

On some architectures like intel accessing unaligned ints is just
slow. On others (Alpha and PPC iirc?) it is an immediate bus error.

I would actually be more curious whether we can do th e comparison
without having to pre-scan for the spaces at the end than trying to
opimize that prescan.

--
greg
http://mit.edu/~gsstark/resume.pdf


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

From
Robert Haas
Date:
On Tue, Jun 16, 2009 at 8:38 AM, Greg Stark<gsstark@mit.edu> wrote:
> On Tue, Jun 16, 2009 at 1:03 PM, Robert Haas<robertmhaas@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.
>
> 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
> clearer even if it's pretty simple.
>
> Incidentally, the char foo[4] = {' ',' ',' ',' '} suggestion is, I
> think, bogus. There would be no alignment guarantee on that array.
> Personally I'm find with 0x20202020 with a comment explaining what it
> is.

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 != ' '.

...Robert


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

From
Andrew Dunstan
Date:

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

andrew


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

From
Jeremy Kerr
Date:
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
> clearer even if it's pretty simple.

Yes, the code (incorrectly) assumes that any multiple-of-4 index into 
the char array is aligned, and so the array itself must be aligned for 
this to work.

I'll rework the patch, testing the pointer alignment directly instead.

> Incidentally, the char foo[4] = {' ',' ',' ',' '} suggestion is, I
> think, bogus. There would be no alignment guarantee on that array.
> Personally I'm find with 0x20202020 with a comment explaining what it
> is.

The variable is called 'spaces', but I can add extra comments if 
preferred.

Cheers,


Jeremy




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

From
Tom Lane
Date:
Greg Stark <gsstark@mit.edu> 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
designers rue the day that their predecessors chose to allow that.)

There are some systems where the kernel trap handler then proceeds to
emulate the unaligned access for you, but that gives new meaning to the
word "slow".  You definitely don't want to be doing it in a patch that's
alleged to give a performance improvement.

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.
        regards, tom lane


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

From
Jeremy Kerr
Date:
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


Uninstallation error

From
Date:
Hi,

I've found the uninstallation error...


# make uninstall

:

n/man7/truncate.7 /usr/local/pgsql/share/man/man7/unlisten.7
/usr/local/pgsql/share/man/man7/update.7
/usr/local/pgsql/share/man/man7/vacuum.7
/usr/local/pgsql/share/man/man7/values.7
rm: cannot remove `/usr/local/pgsql/share/man/man1/': Is a directory
rm: cannot remove `/usr/local/pgsql/share/man/man7/': Is a directory
make[1]: *** [uninstall] Error 1
make[1]: Leaving directory `/usr/local/src/postgresql-8.4rc1/doc'
make: *** [uninstall] Error 2


Maybe, it is solved by the change in the following. (adding -r option)

doc/Makefile
100c100
<     rm -f $(addprefix $(DESTDIR)$(mandir)/, $(shell gunzip -c
$(srcdir)/man.tar.gz | tar tf - | sed -e 's,man7/,man$(sqlmansectnum)/,' -e
's/.7$$/.$(sqlmansect)/'))
---
>     rm -rf $(addprefix $(DESTDIR)$(mandir)/, $(shell gunzip -c
$(srcdir)/man.tar.gz | tar tf - | sed -e 's,man7/,man$(sqlmansectnum)/,' -e
's/.7$$/.$(sqlmansect)/'))


Regards,
Genie Japo



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

From
Stefan Kaltenbrunner
Date:
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.


Stefan


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

From
Chuck McDevitt
Date:
> -----Original Message-----
> From: pgsql-hackers-owner@postgresql.org [mailto:pgsql-hackers-
> owner@postgresql.org] On Behalf Of Stephen Frost
> Sent: Tuesday, June 16, 2009 5:47 AM
> To: Greg Stark
> Cc: Robert Haas; Jeremy Kerr; <pgsql-hackers@postgresql.org>; Alvaro
> Herrera; Stefan Kaltenbrunner; 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
comparing64 bits at a time. 

You might want to consider this.


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

From
Jeremy Kerr
Date:
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 within bcTruelen.

However: Sysbench seems to be quite heavy with the fixed-width char 
types, so may end up calling bcTruelen more than most workloads. Would 
be nice to get some x86 numbers too, but I don't have a suitable machine 
here.

So: The increase in performance is positive on this workload, albeit 
fairly minor. Downside is increased code complexity.

Will re-send the patch once I work out how to get git to create a 
context diff...

Cheers,


Jeremy



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

From
Gurjeet Singh
Date:
On Wed, Jun 17, 2009 at 3:01 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
Will re-send the patch once I work out how to get git to create a
context diff...
--
Lets call it Postgres

EnterpriseDB      http://www.enterprisedb.com

gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | indiatimes | yahoo }.com
Mail sent from my BlackLaptop device

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

From
Heikki Linnakangas
Date:
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 drop in 
> hits within bcTruelen.
> 
> However: Sysbench seems to be quite heavy with the fixed-width char 
> types, so may end up calling bcTruelen more than most workloads. Would 
> be nice to get some x86 numbers too, but I don't have a suitable machine 
> here.
> 
> So: The increase in performance is positive on this workload, albeit 
> fairly minor. Downside is increased code complexity.

Based on this benchmark, I don't think this patch is worth it..

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


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

From
Stefan Kaltenbrunner
Date:
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. Profiles show a decent drop in 
>> hits within .
>>
>> However: Sysbench seems to be quite heavy with the fixed-width char 
>> types, so may end up calling bcTruelen more than most workloads. Would 
>> be nice to get some x86 numbers too, but I don't have a suitable 
>> machine here.
>>
>> So: The increase in performance is positive on this workload, albeit 
>> fairly minor. Downside is increased code complexity.
> 
> Based on this benchmark, I don't think this patch is worth it..

what exactly is getting graphed here - is the what sysbench reports as 
transactions/s or the operations/queries per second? if yes it is 
important to notice that sysnbench by default executes multiple queries 
per transaction and and several different kind of queries. Only some of 
those queries have bcTruelen() showing up on top of the profile.



Stefan


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

From
Gurjeet Singh
Date:
<div dir="ltr"><div class="gmail_quote">On Wed, Jun 17, 2009 at 3:26 PM, Jeremy Kerr <span dir="ltr"><<a
href="mailto:jk@ozlabs.org">jk@ozlabs.org</a>></span>wrote:<br /><blockquote class="gmail_quote" style="border-left:
1pxsolid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> Hurjeet,<br /><div class="im"><br /> >
<ahref="http://wiki.postgresql.org/wiki/Working_with_Git#Context_diffs_with_G"
target="_blank">http://wiki.postgresql.org/wiki/Working_with_Git#Context_diffs_with_G</a><br/> >it<br /><br
/></div>Awesome,thanks.<br /><br /> I'm going to wait for a decision before reformatting and sending the<br /> diff.<br
/></blockquote></div><br/>You might want to submit the patch (if modified from previous submission, which I think it
is),if you want others to benchmark on other archs.<br /><br />Best regards,<br />-- <br />Lets call it Postgres<br
/><br/>EnterpriseDB      <a href="http://www.enterprisedb.com">http://www.enterprisedb.com</a><br /><br
/>gurjeet[.singh]@EnterpriseDB.com<br/>singh.gurjeet@{ gmail | hotmail | indiatimes | yahoo }.com<br />Mail sent from
myBlackLaptop device<br /></div> 

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

From
Stefan Kaltenbrunner
Date:
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 decent drop in 
> hits within bcTruelen.
> 
> However: Sysbench seems to be quite heavy with the fixed-width char 
> types, so may end up calling bcTruelen more than most workloads. Would 
> be nice to get some x86 numbers too, but I don't have a suitable machine 
> here.

This improves my numbers from 46000 queries/s to 52000 queries/s for:

--num-threads=16 --test=oltp --db-ps-mode=auto --pgsql-user=postgres 
--test=oltp --oltp-read-only=on --oltp-sum-ranges=0 
--oltp-simple-ranges=0 --oltp-order-ranges=10 --oltp-point-selects=0 
--oltp-distinct-ranges=0 --oltp-skip-trx=off run

with a profile 16 connections like:

samples  %        symbol name
1925976  23.7136  bcTruelen
329453    4.0564  AllocSetAlloc
292176    3.5974  slot_deform_tuple
278996    3.4351  index_getnext
165397    2.0365  _bt_checkkeys
155621    1.9161  appendBinaryStringInfo
141295    1.7397  LWLockAcquire
136197    1.6769  internal_putbytes
130474    1.6065  AllocSetFree
121341    1.4940  printtup
116413    1.4333  hash_search_with_hash_value
109573    1.3491  FunctionCall2
95101     1.1709  heap_form_minimal_tuple
91475     1.1263  enlargeStringInfo
90841     1.1185  heap_fill_tuple
89926     1.1072  ExecProcNode
86112     1.0603  varstr_cmp

and after the patch applied:

samples  %        symbol name
2225769   7.8378  bcTruelen
1335050   4.7013  index_getnext
1296272   4.5647  AllocSetAlloc
1132026   3.9863  slot_deform_tuple
702219    2.4728  _bt_checkkeys
640675    2.2561  appendBinaryStringInfo
611783    2.1543  LWLockAcquire
599402    2.1107  AllocSetFree
590557    2.0796  internal_putbytes
525526    1.8506  printtup
476077    1.6765  hash_search_with_hash_value
459660    1.6186  FunctionCall2
423569    1.4916  ExecProcNode
391476    1.3785  heap_form_minimal_tuple
385689    1.3582  heap_fill_tuple
383231    1.3495  enlargeStringInfo
368390    1.2973  comparetup_heap
363481    1.2800  varstr_cmp
360261    1.2686  ExecProject
356696    1.2561  pq_putmessage
354193    1.2473  MemoryContextAlloc
341262    1.2017  LWLockRelease
337970    1.1901  pq_begintypsend
321069    1.1306  pfree
310806    1.0945  tuplesort_gettuple_common



Stefan


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

From
Simon Riggs
Date:
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.

Why is bcTruelen being called so many *more* times?

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Training, Services and Support



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

From
Simon Riggs
Date:
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'm looking forward to this improvement as a real-world
gain. (If done right).

I've looked at the code and the thing that bothers me is that I can't
see where or why bcTruelen would be called more often for VARCHAR(n)
than it would be for VARCHAR, on a Select/Sort only workload.

Are we tuning the right thing? Is there some code we can completely
avoid?

If not, does this mean it is a generic effect? Does this imply that
NUMERIC(n) is somehow worse than NUMERIC? etc..

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Training, Services and Support



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

From
Marko Kreen
Date:
On 6/18/09, Simon Riggs <simon@2ndquadrant.com> 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 time that VARCHAR is cheaper than
>  VARCHAR(n), so I'm looking forward to this improvement as a real-world
>  gain. (If done right).
>
>  I've looked at the code and the thing that bothers me is that I can't
>  see where or why bcTruelen would be called more often for VARCHAR(n)
>  than it would be for VARCHAR, on a Select/Sort only workload.

I'd guess plain VARCHAR simply does not have blanks at the end,
so Truelen is cheap.

>  Are we tuning the right thing? Is there some code we can completely
>  avoid?
>
>  If not, does this mean it is a generic effect? Does this imply that
>  NUMERIC(n) is somehow worse than NUMERIC? etc..

Probably not.  For numeric the (n) seems to be only checked input time.

-- 
marko


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

From
Simon Riggs
Date:
On Thu, 2009-06-18 at 15:14 +0300, Marko Kreen wrote:
> On 6/18/09, Simon Riggs <simon@2ndquadrant.com> 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 time that VARCHAR is cheaper than
> >  VARCHAR(n), so I'm looking forward to this improvement as a real-world
> >  gain. (If done right).
> >
> >  I've looked at the code and the thing that bothers me is that I can't
> >  see where or why bcTruelen would be called more often for VARCHAR(n)
> >  than it would be for VARCHAR, on a Select/Sort only workload.
> 
> I'd guess plain VARCHAR simply does not have blanks at the end,
> so Truelen is cheap.

If we were comparing CHAR(n) with VARCHAR then I could agree. But we are
comparing VARCHAR(n) and VARCHAR. There is no blank padding with
VARCHAR, and the two have identical on-disk representations so the cost
of bcTruelen looks like it should be identical in each case.

Is bcTruelen being called too many times, or is there, as Marko
suggests, an explanation as to why calling bcTruelen costs more when we
have a typmod set?

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Training, Services and Support



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

From
Stefan Kaltenbrunner
Date:
Simon Riggs wrote:
> On Thu, 2009-06-18 at 15:14 +0300, Marko Kreen wrote:
>> On 6/18/09, Simon Riggs <simon@2ndquadrant.com> 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 time that VARCHAR is cheaper than
>>>  VARCHAR(n), so I'm looking forward to this improvement as a real-world
>>>  gain. (If done right).
>>>
>>>  I've looked at the code and the thing that bothers me is that I can't
>>>  see where or why bcTruelen would be called more often for VARCHAR(n)
>>>  than it would be for VARCHAR, on a Select/Sort only workload.
>> I'd guess plain VARCHAR simply does not have blanks at the end,
>> so Truelen is cheap.
> 
> If we were comparing CHAR(n) with VARCHAR then I could agree. But we are
> comparing VARCHAR(n) and VARCHAR. There is no blank padding with
> VARCHAR, and the two have identical on-disk representations so the cost
> of bcTruelen looks like it should be identical in each case.

the testcase discusses here is indeed CHAR(n) vs. VARCHAR. To reiterate 
- my numbers(8core/16 thread Nehalem xeon with 16 processes) are 
46000qps for CHAR(n), 52000 qps for CHAR(n) with Jeremys patch(thout 
bcTrueLen is still on top in the profile) and 67000 qps for VARCHAR.


Stefan


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

From
Simon Riggs
Date:
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.comPostgreSQL Training, Services and Support



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

From
Florian Weimer
Date:
* 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 such as this one?

"SELECT c from sbtest where id between $1 and $2 order by c"

(See the start of the thread.)

--
Florian Weimer                <fweimer@bfk.de>
BFK edv-consulting GmbH       http://www.bfk.de/
Kriegsstraße 100              tel: +49-721-96201-1
D-76133 Karlsruhe             fax: +49-721-96201-99


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

From
Tom Lane
Date:
Simon Riggs <simon@2ndQuadrant.com> 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
storage) to figure out what the "valid" string length is for comparing.

varchar considers any trailing blanks to be real, comparable data,
so it simply hasn't got any equivalent code.

It would be way nicer if we could strip trailing blanks on storage,
and then figure a way to either reconstitute them on output (problem
here is the output function doesn't have access to typmod) or
language-lawyer our way to deciding we don't have to.
        regards, tom lane


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

From
"Kevin Grittner"
Date:
Tom Lane <tgl@sss.pgh.pa.us> 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?
-Kevin


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

From
Simon Riggs
Date:
On Thu, 2009-06-18 at 09:59 -0400, Tom Lane wrote:
> Simon Riggs <simon@2ndQuadrant.com> 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 trailing blanks on storage,
> and then figure a way to either reconstitute them on output (problem
> here is the output function doesn't have access to typmod) or
> language-lawyer our way to deciding we don't have to.

Is there a spare bit on the varhdr that can be set by datatypes? It
would be useful to have a bit meaning "there is a typmod stored on this
Datum", that would allow the output function to do some special
processing. 

I notice we lose on tuple access also. CHAR(n) is fixed length, but is
treated as variable length for offsets.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Training, Services and Support



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

From
Alvaro Herrera
Date:
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 Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


Re: Uninstallation error

From
Peter Eisentraut
Date:
On Tuesday 16 June 2009 18:16:58 genie.japo@gmail.com wrote:
> Hi,
>
> I've found the uninstallation error...
>
>
> # make uninstall
>
>
>
> n/man7/truncate.7 /usr/local/pgsql/share/man/man7/unlisten.7
> /usr/local/pgsql/share/man/man7/update.7
> /usr/local/pgsql/share/man/man7/vacuum.7
> /usr/local/pgsql/share/man/man7/values.7
> rm: cannot remove `/usr/local/pgsql/share/man/man1/': Is a directory
> rm: cannot remove `/usr/local/pgsql/share/man/man7/': Is a directory
> make[1]: *** [uninstall] Error 1
> make[1]: Leaving directory `/usr/local/src/postgresql-8.4rc1/doc'
> make: *** [uninstall] Error 2
>
>
> Maybe, it is solved by the change in the following. (adding -r option)
>
> doc/Makefile
> 100c100
> <     rm -f $(addprefix $(DESTDIR)$(mandir)/, $(shell gunzip -c
> $(srcdir)/man.tar.gz | tar tf - | sed -e 's,man7/,man$(sqlmansectnum)/,' -e
> 's/.7$$/.$(sqlmansect)/'))
> ---
>
> >     rm -rf $(addprefix $(DESTDIR)$(mandir)/, $(shell gunzip -c
>
> $(srcdir)/man.tar.gz | tar tf - | sed -e 's,man7/,man$(sqlmansectnum)/,' -e
> 's/.7$$/.$(sqlmansect)/'))

I have fixed this problem.

Next time, please start a new thread for a new issue, so your issue doesn't 
get lost.


Re: Uninstallation error

From
Alvaro Herrera
Date:
Peter Eisentraut escribió:

> Next time, please start a new thread for a new issue, so your issue doesn't 
> get lost.

Gmail has gotten the idiotic idea that changing the subject makes the
message appear as a new thread :-(

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


Re: Uninstallation error

From
Date:
> I have fixed this problem.

Thanks a lot for the fix !

>> Next time, please start a new thread for a new issue, so your issue
doesn't 
>> get lost.
> Gmail has gotten the idiotic idea that changing the subject makes the
> message appear as a new thread :-(

Sorry.
This is the first posting in hackers-ML for me.
I don't know why this issue occurs.
Gmail is a little funny :~(~~~

Regards,
Genie Japo



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

From
Tom Lane
Date:
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Tom Lane <tgl@sss.pgh.pa.us> 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?

I think that's what I said.  AFAIK there isn't any place where we'd
need to add back the spaces except the output function.  All the
operators would be just as happy if the spaces weren't there.
        regards, tom lane


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

From
"Kevin Grittner"
Date:
Tom Lane <tgl@sss.pgh.pa.us> wrote: 
> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
>> Tom Lane <tgl@sss.pgh.pa.us> 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?
> 
> I think that's what I said.  AFAIK there isn't any place where we'd
> need to add back the spaces except the output function.  All the
> operators would be just as happy if the spaces weren't there.
Sorry, I misread it.
If this can be done, it might ease the transition from other products
for some people.  In Sybase there were significant performance
benefits to using char() instead of varchar() in certain
circumstances.  In our first test conversion from Sybase we mapped all
domains and columns which were char(n) in Sybase to the same in
PostgreSQL, which caused some pain.  It was no big deal to remap those
to varchar() and re-run, but on the face of it, it seems like there
wouldn't be a significant performance hit for char() with this change,
so it would be one less thing for people to stub their toes on during
migration.
-Kevin


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

From
Simon Riggs
Date:
On Thu, 2009-06-18 at 12:58 -0400, Tom Lane wrote:
> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> > Tom Lane <tgl@sss.pgh.pa.us> 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?
> 
> I think that's what I said.  AFAIK there isn't any place where we'd
> need to add back the spaces except the output function.  All the
> operators would be just as happy if the spaces weren't there.

The overall problem is that we expect the Datum's of a datatype to know
how to display themselves without any access to metadata.

Another way of looking at this might be that we need a default FORMAT
specifier associated with a column. Teradata used the FORMAT specifier
on a column to allow you to specify a default format. That allowed you
to specify leading/trailing zeros/spaces, decimal points and other
characters.

It would be good to have a generic way to specify formatting to a data
type at the point you are manipulating it, rather than globally.

That would be helpful because we currently have some very ugly things
like the DateStyle parameter. That effects the Input and Output of data
into Time/Date columns, forcing you to switch parameter values
constantly as you load/select data. The current way makes it impossible
to load data that has different date formats in different columns, for
example.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Training, Services and Support



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

From
Greg Stark
Date:
On Thu, Jun 18, 2009 at 10:03 PM, Simon Riggs<simon@2ndquadrant.com> 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 column. Teradata used the FORMAT specifier
> on a column to allow you to specify a default format. That allowed you
> to specify leading/trailing zeros/spaces, decimal points and other
> characters.

I've suggested this in the past in the context of getting rid of the
"cash" data type.

However I'm not sure it solves this problem. In the case where we're
just outputing a column we could arrange to have the typmod pretty
easily. but what would you do if you had some complex expression which
happened to result in a char(). Say something like:

coalesce(nickname, name)

where both nickname and name are, say, char(50). Or for that matter
where they're two different length chars, like char(50) and char(100).

<brainstorming>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. Perhaps. I think we
already have rtrim as the cast in one direction and I think we already
have operators which is the whole trigger for the bcTrueLen thing
which started this thread. maybe it wouldn't really be that much of a
pain.</brainstorming>


-- 
greg
http://mit.edu/~gsstark/resume.pdf


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

From
Tom Lane
Date:
Greg Stark <gsstark@mit.edu> writes:
> <brainstorming>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 compatible with text.  The space hit would
be a bigger issue ... at least for cases where you don't save much of
anything by eliminating the spaces, which would be exactly the cases
where sane Postgres users use char(n) today.
        regards, tom lane


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

From
Jeremy Kerr
Date:
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 number of compares where possible. Word-size compares
are performed on aligned sections of the string.

Results in a small performance increase; around 1-2% on my POWER6 test
box.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

---
Resend: context diff this time

---src/backend/utils/adt/varchar.c |   24 +++++++++++++++++++++---1 file changed, 21 insertions(+), 3 deletions(-)

*** a/src/backend/utils/adt/varchar.c
--- b/src/backend/utils/adt/varchar.c
***************
*** 624,639 **** varchartypmodout(PG_FUNCTION_ARGS) static int bcTruelen(BpChar *arg) {     char       *s =
VARDATA_ANY(arg);    int            i;
 
-     int            len; 
!     len = VARSIZE_ANY_EXHDR(arg);
!     for (i = len - 1; i >= 0; i--)     {         if (s[i] != ' ')             break;     }     return i + 1; } 
--- 624,657 ---- static int bcTruelen(BpChar *arg) {
+     const uint32_t    spaces = 0x20202020;
+     const int    wordsize = sizeof(spaces);     char       *s = VARDATA_ANY(arg);     int            i; 
!     i = VARSIZE_ANY_EXHDR(arg) - 1;
! 
!     /* compare down to an aligned boundary */
!     for (; i > 0 && !PointerIsAligned(s + i - (wordsize - 1), uint32_t); i--)     {         if (s[i] != ' ')
+             return i + 1;
+     }
+ 
+     /* now that we're aligned, compare word at a time */
+     for (; i >= wordsize - 1; i -= wordsize)
+     {
+         if (*(uint32_t *)(s + i - (wordsize - 1)) != spaces)             break;     }
+ 
+     /* check within the last non-matching word */
+     for (; i >= 0; i--)
+     {
+         if (s[i] != ' ')
+             break;
+     }
+      return i + 1; } 


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

From
Robert Haas
Date:
On Tue, Jun 23, 2009 at 9:05 PM, Jeremy Kerr<jk@ozlabs.org> 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


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

From
Jeremy Kerr
Date:
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



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

From
Robert Haas
Date:
On Tue, Jun 23, 2009 at 10:46 PM, Jeremy Kerr<jk@ozlabs.org> 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 original message.

You probably want to add your patch here:
http://wiki.postgresql.org/wiki/CommitFestOpen

Thanks,

...Robert


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

From
Stefan Kaltenbrunner
Date:
Robert Haas wrote:
> On Tue, Jun 23, 2009 at 10:46 PM, Jeremy Kerr<jk@ozlabs.org> 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 original message.
> 
> You probably want to add your patch here:
> http://wiki.postgresql.org/wiki/CommitFestOpen

FWIW: I'm able to measure an even more significant improvement of around 
10%:

http://archives.postgresql.org/pgsql-hackers/2009-06/msg01041.php on a 
specific query.


Stefan


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

From
Stephen Frost
Date:
* Stefan Kaltenbrunner (stefan@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 full benchmark runs since those can show minimal
performance improvments due to other constraints, locking, etc).

Have you tried pulling this function out and running tests with all
alignment possibilities to see how it compares to the original?  There's
only so many options and showing that this change always improves
performance, or at least doesn't degrade it in the worst case, would
certainly go a long way towards getting it accepted..
Thanks,
    Stephen

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

From
Stefan Kaltenbrunner
Date:
Stephen Frost wrote:
> * Stefan Kaltenbrunner (stefan@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 full benchmark runs since those can show minimal
> performance improvments due to other constraints, locking, etc).

not sure what you are after here - my testcase is one specific query run 
in parallel on 16 processes (running it in a single process yields 
similiar improvements our scaling is pretty good here).

> 
> Have you tried pulling this function out and running tests with all
> alignment possibilities to see how it compares to the original?  There's
> only so many options and showing that this change always improves
> performance, or at least doesn't degrade it in the worst case, would
> certainly go a long way towards getting it accepted.

again I'm not exactly sure on what you want to get tested specifically - 
the original problem report was because I noticed a significant 
performance improvement from using varchar() instead of char(n) and that 
showed bcTruelen() on the very top of the profile.
I was simply testing the patch Jeremy provided on that workload(the 
upthread mentioned query is mostly affected by that on others there is 
no measurable difference)


Stefan


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

From
Stephen Frost
Date:
Stefan,

* Stefan Kaltenbrunner (stefan@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 minimal
>> performance improvments due to other constraints, locking, etc).
>
> not sure what you are after here - my testcase is one specific query run
> in parallel on 16 processes (running it in a single process yields
> similiar improvements our scaling is pretty good here).

What I'm suggesting is to test what happens when strings sent to
bcTruelen are at different memory addresses (there's only 4 or 8
different possibilities here, then different lengths should be tested of
less-than-1-word, 1-word, >1-word, and then different ending points at
different memory addresses) and compare the new function to the old.

The main point here being to try and find out if there are any
pathological cases where the new function is much worse, or even
somewhat worse, so we can weigh that against the places where it
performs better.

This isn't something you can do from in PG. :)

> I was simply testing the patch Jeremy provided on that workload(the
> upthread mentioned query is mostly affected by that on others there is
> no measurable difference)

Certainly, your performance numbers are good justification if there's no
downside.
Thanks,
    Stephen

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

From
Jeremy Kerr
Date:
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 theoretically one quarter of cost (a test benchmark 
on x86 shows an actual reduction of 11 to 3 sec with a string of 100 
spaces).

Worst-case behaviour is with smaller numbers of spaces. Here are the 
transition points (ie, where doing the word-wise comparison is faster 
than byte-wise) that I see from my benchmark:
align    spaces    3    7    2    6    1    5    0    4
- where 'align' is the alignment of the first byte to compare (ie, at 
the end of the string). This is pretty much as-expected, as these 
transition points are the first opportunity that the new function has to 
do a word compare.

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 number of padding spaces we'd expect to see on 
workload data. Fortunately, we see the larger reductions on the more 
expensive operations (ie, longer strings).

Cheers,


Jeremy


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

From
Tom Lane
Date:
Jeremy Kerr <jk@ozlabs.org> 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 number of padding spaces we'd expect to see on 
> workload data. Fortunately, we see the larger reductions on the more 
> expensive operations (ie, longer strings).

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) being used properly, such as char(2)
for US state abbreviations.
        regards, tom lane


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

From
Jeremy Kerr
Date:
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 benchmark I've been using:
http://ozlabs.org/~jk/projects/db/bctrulen.tar.gz

to recreate as-needed. ./benchmark.sh will run tests for varying-length 
strings (so changing the alignment) and varying numbers of trailing 
spaces. ./benchmark-bctruelen <str> will perform an individual old/new 
comparison.

> 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) being used properly,
> such as char(2) for US state abbreviations.

Yep, makes sense. The other consideration is stock-ticker symbols, I 
assume they may also be stored in CHAR(small n) columns.

Cheers,


Jeremy


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

From
Dimitri Fontaine
Date:
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) being used properly,
>> such as char(2) for US state abbreviations.
>
> Yep, makes sense. The other consideration is stock-ticker symbols, I
> assume they may also be stored in CHAR(small n) columns.


Could this optimisation only kicks in when n is "big enough"?
I'm don't know if this part of the code knows the typmod...

Regards,
--
dim

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

From
Stephen Frost
Date:
* Dimitri Fontaine (dfontaine@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 performance
>>> degradation for a "canonical" example of char(n) being used properly,
>>> such as char(2) for US state abbreviations.
>>
>> Yep, makes sense. The other consideration is stock-ticker symbols, I
>> assume they may also be stored in CHAR(small n) columns.
>
> Could this optimisation only kicks in when n is "big enough"?
> I'm don't know if this part of the code knows the typmod...

Is it just the size that matters, or is it when there are few spaces at
the end?  We do know the overall length, but I didn't see a definite
that if it's larger than X words, doing the by-word comparison is a win
regardless of how many actual spaces are at the end (apologies to Jeremy
if it's in his more detailed report, I havn't had a chance to look yet).
Thanks,
    Stpehen

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

From
Jeremy Kerr
Date:
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


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

From
Stephen Frost
Date:
* Jeremy Kerr (jk@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 of this function..  We
know the total length of the string, including spaces, coming into the
function.  If the updated function is always faster when the overall
string is at least, say, 16 characters long, then Dimitri's suggestion
to just check the overall length coming in and decide which approach to
use might make sense.  If the new function is always slower, regardless
of overall string length, when there's only 1 extra space at the end,
then chances are it's not worth it.
Thanks,
    Stephen

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

From
Jeremy Kerr
Date:
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 be faster, we need to know that there are more 
than 6 (on average, depending on alignment) trailing spaces. The number 
of non-space characters in the string won't affect performance (other 
than that there is probably a correlation between total string length 
and number of spaces, but we can't be certain of that).

> If the new function is always slower, regardless of overall string
> length, when there's only 1 extra space at the end

Yes, that is correct.

Cheers,


Jeremy



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

From
Tom Lane
Date:
Jeremy Kerr <jk@ozlabs.org> 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 end.

Right, but there are certainly not more spaces than there are string
characters ;-)

I think Dimitri's idea is eminently worth trying.  In a string of less
than, say, 16 bytes, the prospects of being able to win anything get
much smaller compared to the prospects of wasting the extra loop
overhead.  There is also a DBA psychology angle to it.  If you've got
CHAR(n) for very small n, it's likely that the type is being used in the
"canonical" fashion and there won't be many trailing blanks.  The case
where we can hope to win is where we have CHAR(255) or some other
plucked-from-the-air limit.
        regards, tom lane


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

From
Dimitri Fontaine
Date:
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-6] == 0x20, àla
Boyer/Moore?

I call it tricky because you could have a space here which isn't
followed by spaces, but still, if it's not a space here, you're saying
we should not even try the optimisation.

--
dim

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

From
tomas@tuxteam.de
Date:
-----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.
>
> 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 aligned word which completely
fits in the buffer? Something along the lines of (assuming four-byte
words)
  * (int*) (4 * ((int) &buf[0]) / 4)

(now that's an ugly one, but you know what I mean?)

Regards
- -- tomás
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFKROlCBcgs9XrR2kYRArHPAJ9VhT+RfK5/5BxwA0nxaOmK4nfuWACdFtFL
iKtvPaZY/KhDJMOf4hyzmQI=
=yd05
-----END PGP SIGNATURE-----


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

From
Tom Lane
Date:
tomas@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 aligned word which completely
> fits in the buffer? Something along the lines of (assuming four-byte
> words)
>    * (int*) (4 * ((int) &buf[0]) / 4)

We're trying to avoid adding cycles to the optimization-is-useless case.
The more expensive this test gets, the slower the unoptimizable case
becomes.
        regards, tom lane


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

From
"Andrew Dunstan"
Date:
On Fri, June 26, 2009 11:39 am, Tom Lane wrote:
> tomas@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 aligned word which completely
>> fits in the buffer? Something along the lines of (assuming four-byte
>> words)
>>    * (int*) (4 * ((int) &buf[0]) / 4)
>
> We're trying to avoid adding cycles to the optimization-is-useless case.
> The more expensive this test gets, the slower the unoptimizable case
> becomes.
>

Yeah. Like you, I like the idea of a switch based on string length. I
would suggest a cutoff of something like 36 (length of the string
representation of a UUID). But maybe that will miss lots of optimisable
cases like address fields. I guess those people should really be using
varchar(n) anyway.

cheers

andrew




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

From
Bruce Momjian
Date:
Tom Lane wrote:
> Jeremy Kerr <jk@ozlabs.org> 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 end.
> 
> Right, but there are certainly not more spaces than there are string
> characters ;-)
> 
> I think Dimitri's idea is eminently worth trying.  In a string of less
> than, say, 16 bytes, the prospects of being able to win anything get
> much smaller compared to the prospects of wasting the extra loop
> overhead.  There is also a DBA psychology angle to it.  If you've got
> CHAR(n) for very small n, it's likely that the type is being used in the
> "canonical" fashion and there won't be many trailing blanks.  The case
> where we can hope to win is where we have CHAR(255) or some other
> plucked-from-the-air limit.

What ever happened to this patch?

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.comPG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard
drive,Christ can be your backup. +
 


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

From
Robert Haas
Date:
On Mon, Feb 22, 2010 at 9:35 PM, Bruce Momjian <bruce@momjian.us> wrote:
> Tom Lane wrote:
>> Jeremy Kerr <jk@ozlabs.org> 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 end.
>>
>> Right, but there are certainly not more spaces than there are string
>> characters ;-)
>>
>> I think Dimitri's idea is eminently worth trying.  In a string of less
>> than, say, 16 bytes, the prospects of being able to win anything get
>> much smaller compared to the prospects of wasting the extra loop
>> overhead.  There is also a DBA psychology angle to it.  If you've got
>> CHAR(n) for very small n, it's likely that the type is being used in the
>> "canonical" fashion and there won't be many trailing blanks.  The case
>> where we can hope to win is where we have CHAR(255) or some other
>> plucked-from-the-air limit.
>
> 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 gains to no obvious benefit at
all, and it's very unclear that anyone has made a serious effort to
find a benchmark the worst-case scenarios.  I think we should drop
this for now.  *If* someone wants to put some work into more thorough
analysis for 9.1, we can revisit it then.

...Robert


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

From
Alvaro Herrera
Date:
Robert Haas escribió:
> On Mon, Feb 22, 2010 at 9:35 PM, Bruce Momjian <bruce@momjian.us> 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 gains to no obvious benefit at
> all, and it's very unclear that anyone has made a serious effort to
> find a benchmark the worst-case scenarios.  I think we should drop
> this for now.  *If* someone wants to put some work into more thorough
> analysis for 9.1, we can revisit it then.

+1 -- it's not like it hasn't been a problem all along.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


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

From
Stefan Kaltenbrunner
Date:
Alvaro Herrera wrote:
> Robert Haas escribió:
>> On Mon, Feb 22, 2010 at 9:35 PM, Bruce Momjian <bruce@momjian.us> 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 gains to no obvious benefit at
>> all, and it's very unclear that anyone has made a serious effort to
>> find a benchmark the worst-case scenarios.  I think we should drop
>> this for now.  *If* someone wants to put some work into more thorough
>> analysis for 9.1, we can revisit it then.
> 
> +1 -- it's not like it hasn't been a problem all along.

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 especially 
for migrated database is imho not true) it is an issue that can be seen 
with a rather popular database related benchmarking tool so we should 
not really dismiss it easily.


Stefan


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

From
Tom Lane
Date:
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> 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 especially 
> for migrated database is imho not true) it is an issue that can be seen 
> with a rather popular database related benchmarking tool so we should 
> not really dismiss it easily.

Nobody suggested dismissing it.  The point was that it hasn't been
tested adequately to justify applying it now.
        regards, tom lane


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

From
Stefan Kaltenbrunner
Date:
Tom Lane wrote:
> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> 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 especially 
>> for migrated database is imho not true) it is an issue that can be seen 
>> with a rather popular database related benchmarking tool so we should 
>> not really dismiss it easily.
> 
> 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)?


Stefan


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

From
Tom Lane
Date:
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> 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 was complaining that the worst case hadn't been characterized
adequately, which I agree with.  We know it helps a lot on certain
cases, but what's the downside?
        regards, tom lane


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

From
Bruce Momjian
Date:
Tom Lane wrote:
> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> 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 was complaining that the worst case hadn't been characterized
> adequately, which I agree with.  We know it helps a lot on certain
> cases, but what's the downside?

Added to TODO list so we don't forget about it:
Considering improving performance of computing CHAR() value lengths

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.comPG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard
drive,Christ can be your backup. +