forked frompostgres/postgres
- Notifications
You must be signed in to change notification settings - Fork6
Commit607f8ce
committed
Avoid a performance regression in float overflow/underflow detection.
Commit6bf0bc8 replaced float.c's CHECKFLOATVAL() macro with staticinline subroutines, but that wasn't too well thought out. In the originalcoding, the unlikely condition (isinf(result) or result == 0) was checkedfirst, and the inf_is_valid or zero_is_valid condition only afterwards.The inline-subroutine coding caused that to be swapped around, which ispretty horrid for performance because (a) in common cases the is_validcondition is twice as expensive to evaluate (e.g., requiring two isinf()calls not one) and (b) in common cases the is_valid condition is false,requiring us to perform the unlikely-condition check anyway. Net resultis that one isinf() call becomes two or three, resulting in visibleperformance loss as reported by Keisuke Kuroda.The original fix proposal was to revert the replacement of the macro,but on second thought, that macro was just a bad idea from the beginning:if anything it's a net negative for readability of the code. So instead,let's just open-code all the overflow/underflow tests, being careful totest the unlikely condition first (and mark it unlikely() to help thecompiler get the point).Also, rather than having N copies of the actual ereport() calls, collapsethose into out-of-line error subroutines to save some code space. Thisdoes mean that the error file/line numbers won't be very helpful forfiguring out where the issue really is --- but we'd already burned thatbridge by putting the ereports into static inlines.In HEAD, check_float[48]_val() are gone altogether. In v12, leave thempresent in float.h but unused in the core code, just in case someextension is depending on them.Emre Hasegeli, with some kibitzing from me and Andres FreundDiscussion:https://postgr.es/m/CANDwggLe1Gc1OrRqvPfGE=kM9K0FSfia0hbeFCEmwabhLz95AA@mail.gmail.com1 parentcaba091 commit607f8ce