Log In - Home Page

Patch: Avoid manual shift-and-test logic in AllocSetFreeIndex

Edit Patch - Move To Another CommitFest - Delete Patch

CommitFest 2009-07
Topic Performance
Patch Status Committed
Author Jeremy Kerr
Reviewers Dan Colish
Committer Nobody
Close Date 2009-07-21
Comments
Patch by jk- on 2009-06-30 07:08:17 AM: Initial version
Comment by dcolish on 2009-07-15 02:22:50 PM: Applied and built cleanly. Regress passes. Trying to hunt down ppc box to see if performance enhancement can be seen.
Comment by hlipa on 2009-07-16 06:03:47 AM: I have several objections:

- inline is forbidden to use in PostgreSQL - you need exception or do it differently

- type mismatch - Size vs unsigned int vs 32. you should use only Size and sizeof(Size)

And general comment:

Do we want to have this kind of code which is optimized for one compiler and platform. See openSSL or MySQL, they have many optimizations which work fine on one platform with specific version of compiler and specific version of OS. But if you select different version of compiler or different compiler you can get very different performance result (e.g. MySQL 30% degradation, OpenSSL RSA 3x faster and so on).

I think in this case, it makes sense to have optimization here, but we should do it like spinlocks are implemented and put this code into /port. It should help to implemented special code for SPARC and SUN Studio for example.


Zdenek
Comment by rhaas on 2009-07-16 07:45:12 AM: Please don't post reviewers here. Post reviews to -hackers and link here.
Comment by rhaas on 2009-07-19 05:30:30 AM: Is this PPC-specific? Also, need input from author on Zdenek's concerns.
Patch by tgl on 2009-07-20 09:05:14 AM: v4 of patch, in response to various comments
Review by tgl on 2009-07-20 10:54:52 AM: Actually this doesn't look like it's such a win after all ...
Comment by tgl on 2009-07-21 12:44:21 PM: Current plan is to use something based on AllocSetFreeIndex_lts as shown here
Patch by tgl on 2009-07-21 02:02:18 PM: Version as committed

Add Comment

Please log in to comment on this patch.