refactor heap_deform_tuple guts
Hi,
heap_deform_tuple and slot_deform_tuple contain duplicated code. This
patch refactors them so that the guts are in a single place.
I have checked the resulting assembly code for heap_deform_tuple, and
with the "inline" declaration, the gcc version I have (4.7.2) generates
almost identical output both after the patch than before, thus there
shouldn't be any slowdown.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
deconstruct-tuple.patchtext/x-diff; charset=us-asciiDownload+107-100
On Tue, Aug 6, 2013 at 6:32 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
heap_deform_tuple and slot_deform_tuple contain duplicated code. This
patch refactors them so that the guts are in a single place.I have checked the resulting assembly code for heap_deform_tuple, and
with the "inline" declaration, the gcc version I have (4.7.2) generates
almost identical output both after the patch than before, thus there
shouldn't be any slowdown.
Although I'm generally in favor of eliminating duplicated code, I have
to admit that in this case I'm not sure I see the point.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas escribi�:
On Tue, Aug 6, 2013 at 6:32 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
heap_deform_tuple and slot_deform_tuple contain duplicated code. This
patch refactors them so that the guts are in a single place.I have checked the resulting assembly code for heap_deform_tuple, and
with the "inline" declaration, the gcc version I have (4.7.2) generates
almost identical output both after the patch than before, thus there
shouldn't be any slowdown.Although I'm generally in favor of eliminating duplicated code, I have
to admit that in this case I'm not sure I see the point.
Yeah, I guess in isolation this doesn't make that much sense. I am
hesitant to create a third copy in the minmax patch, but I will do that
for now and propose the refactoring later.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2013-08-07 10:36:52 -0400, Alvaro Herrera wrote:
Robert Haas escribi�:
On Tue, Aug 6, 2013 at 6:32 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
heap_deform_tuple and slot_deform_tuple contain duplicated code. This
patch refactors them so that the guts are in a single place.I have checked the resulting assembly code for heap_deform_tuple, and
with the "inline" declaration, the gcc version I have (4.7.2) generates
almost identical output both after the patch than before, thus there
shouldn't be any slowdown.Although I'm generally in favor of eliminating duplicated code, I have
to admit that in this case I'm not sure I see the point.Yeah, I guess in isolation this doesn't make that much sense. I am
hesitant to create a third copy in the minmax patch, but I will do that
for now and propose the refactoring later.
Well, you didn't mention upthread that you want to do this because
you're going to need another variant of the same code. Imo that's
sufficient reasoning.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund escribi�:
On 2013-08-07 10:36:52 -0400, Alvaro Herrera wrote:
Yeah, I guess in isolation this doesn't make that much sense. I am
hesitant to create a third copy in the minmax patch, but I will do that
for now and propose the refactoring later.Well, you didn't mention upthread that you want to do this because
you're going to need another variant of the same code. Imo that's
sufficient reasoning.
Well, I would need to tweak the new code again, moving it from
heaptuple.c to htup_details.h as a STATIC_IF_INLINE function; and
furthermore I have noticed that if I modify the code in minmax, I can
have it do one more thing that I would need to do outside of it anyway.
(These tuples have two null bitmaps; I would have to decode the second
one separately. Doing it inside the new "deform" variant is much
easier.)
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers