Re: refactoring - share str2*int64 functions
От | Fabien COELHO |
---|---|
Тема | Re: refactoring - share str2*int64 functions |
Дата | |
Msg-id | alpine.DEB.2.21.1909011914520.11815@lancre обсуждение исходный текст |
Ответ на | Re: refactoring - share str2*int64 functions (Michael Paquier <michael@paquier.xyz>) |
Ответы |
Re: refactoring - share str2*int64 functions
(Michael Paquier <michael@paquier.xyz>)
|
Список | pgsql-hackers |
Michaël, >> I would put back unlikely() on overflow tests, as there are indeed unlikely >> to occur and it may help some compilers, and cannot be harmful. It also >> helps the code reader to know that these path are not expected to be taken >> often. > > Hm. I don't agree about that part, and the original signed portions > don't do that. I think that we should let the callers of the routines > decide if a problem is unlikely to happen or not as we do now. Hmmm. Maybe inlining propates them, but otherwise they make sense for a compiler perspective. > I am still not convinced that this module is worth the extra cycles to > justify its existence though. They allow to quickly do performance tests, for me it is useful to keep it around, but you are the committer, you do as you feel. >> [...] >> There is no doubt that dividing 64 bits integers is a very bad idea, at >> least on my architecture! > > That's surprising. I cannot reproduce that. It seems to me that somehow you can, you have a 5 to 18 seconds drop below. There are actual reasons why some processors are more expensive than others, it is not just marketing:-) > 2-1) mul: > - built-in: 5.1s > - fallback (with uint128): 8.0s > - fallback (without uint128): 18.1s > So, the built-in option is always faster, and keeping the int128 path > if available for the multiplication makes sense, but not for the > subtraction and the addition. Yep. > I am wondering if we should review further the signed part for add and > sub, but I'd rather not touch it in this patch. The signed overflows are trickier even, I have not paid attention to the fallback code. I agree that it is better left untouched for know. > If you have done any changes on my previous patch, or if you have a > script to share I could use to attempt to reproduce your results, I > would be happy to do so. Hmmm. I did manual tests really. Attached a psql script replicating them. # with builtin overflow detection sh> psql < oc.sql NOTICE: int 16 mul: 00:00:02.747269 # slow NOTICE: int 16 add: 00:00:01.83281 NOTICE: int 16 sub: 00:00:01.8501 NOTICE: uint 16 mul: 00:00:03.68362 # slower NOTICE: uint 16 add: 00:00:01.835294 NOTICE: uint 16 sub: 00:00:02.136895 # slow NOTICE: int 32 mul: 00:00:01.828065 NOTICE: int 32 add: 00:00:01.840269 NOTICE: int 32 sub: 00:00:01.843557 NOTICE: uint 32 mul: 00:00:02.447052 # slow NOTICE: uint 32 add: 00:00:01.849899 NOTICE: uint 32 sub: 00:00:01.840773 NOTICE: int 64 mul: 00:00:01.839051 NOTICE: int 64 add: 00:00:01.839065 NOTICE: int 64 sub: 00:00:01.838599 NOTICE: uint 64 mul: 00:00:02.161346 # slow NOTICE: uint 64 add: 00:00:01.839404 NOTICE: uint 64 sub: 00:00:01.838549 DO > So, do you have more comments? No other comments. -- Fabien.
Вложения
В списке pgsql-hackers по дате отправления: