Thread
-
REVIEW: Optimizing box_penalty
Kevin Grittner <kevin.grittner@wicourts.gov> — 2011-10-08T18:51:11Z
I tried to review the "Optimizing box_penalty" patch: https://commitfest.postgresql.org/action/patch_view?id=600 as posted here: http://archives.postgresql.org/message-id/4E088690.5080706@enterprisedb.com The patch no longer applies to source code, due to other recent GiST changes. Parts of it were modifying functions which no longer exist. I picked out the bits which still seemed relevant, and a patch with that is attached. The improvement in REINDEX time is now only about 0.25% on my machine at home, which is small enough that it could easily be from code shifting around. (Or such shifts might be hiding a larger actual savings.) In any event, this patch doesn't seem to be justified as a performance patch, based on my benchmarks today. On the other hand, this patch leaves the code a few lines shorter and eliminates some unnecessary Datum wrapping, PG_FUNCTION_ARGS parameters on a static function, and allows that function to be called directly rather than using DirectFunctionCall2(). I find the resulting code a little cleaner and easier to read. I would prefer to see it applied on that basis, personally. Since the author is a committer, and this is a pretty minor code style patch at this point, I'll mark it "Ready for Committer" and leave it to Heikki to decide what to do with it. -Kevin
-
Re: REVIEW: Optimizing box_penalty
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2011-10-09T16:01:07Z
On 08.10.2011 21:51, Kevin Grittner wrote: > I tried to review the "Optimizing box_penalty" patch: Thanks! > On the other hand, this patch leaves the code a few lines shorter and > eliminates some unnecessary Datum wrapping, PG_FUNCTION_ARGS > parameters on a static function, and allows that function to be > called directly rather than using DirectFunctionCall2(). I find the > resulting code a little cleaner and easier to read. I would prefer > to see it applied on that basis, personally. Agreed, committed. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com