Lists: | pgsql-hackers |
---|
From: | "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> |
---|---|
To: | <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
Cc: | <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Optimizing pg_trgm makesign() (was Re: WIP: Fast GiST index build) |
Date: | 2011-09-25 22:30:18 |
Message-ID: | 4E7F652A02000025000416DB@gw.wicourts.gov |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
This is a review of the patch at this CF location:
https://commitfest.postgresql.org/action/patch_view?id=598
as posted here:
http://archives.postgresql.org/message-id/4E04C099.3020604@enterprisedb.com
This patch applied cleanly and compiled without warning. It
performed correctly. Since the patch modifies one function which has
one input and one output (through a pointer parameter), with no other
side effects, it has low risk of surprising problems.
It is strictly a performance patch, and accomplishes its goals. I
ran the entire dictionary through the patched and unpatched versions
five times each (using the supplied debugging patch) and saw no
changes to the behavior of the function -- identical results every
time. I ran performance tests with building and reindexing the
sorted dictionary, a randomly ordered dictionary, a randomly ordered
dictionary with the entries randomly truncated to 0 to 3 characters,
and a large README file -- five times each with and without the
patch. The patch was a clear winner with all of those except the
truncated dictionary, where the difference we well within the noise
(0.05% difference summing five runs when individual runs could vary
by up to about 4%).
While there was no reason to believe it could affect search
performance, I tested that anyway, and found no difference.
Once this message shows up on the list (so I can retrieve the message
ID) I will mark this "Ready for Committer".
-Kevin
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> |
Cc: | heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Re: Optimizing pg_trgm makesign() (was Re: WIP: Fast GiST index build) |
Date: | 2011-09-26 01:43:40 |
Message-ID: | 12429.1317001420@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> This is a review of the patch at this CF location:
> https://commitfest.postgresql.org/action/patch_view?id=598
> as posted here:
> http://archives.postgresql.org/message-id/4E04C099.3020604@enterprisedb.com
Hmm, why is that patch the one posted for review, when several better
versions were already discussed? See thread starting here:
http://archives.postgresql.org/pgsql-hackers/2011-07/msg00028.php
regards, tom lane