On Sat, Sep 26, 2020 at 2:17 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > That led me to refactor the patch as attached. (I'd first thought ...
Thanks for refactoring the patch. Everything looks good to me. As expected, observations for the v02 patch gave pretty much the same results as v01 (previous mail). v02 perf results 2.07% GetSQLCurrentTime 0.50% GetSQLCurrentDate --.-% GetSQLLocalTime 0.69% GetCurrentDateTime -.--% GetCurrentTimeUsec v02 elapsed time Run1 1m38s Run2 1m35s Run3 1m33s (gives same %19 improvement as v01) ~ I only have a couple of questions, more for curiosity than anything else. 1. Why is there sometimes an extra *tm = &tt; variable introduced? (e.g. GetSQLCurrentTime, GetSQLLocalTime). Why not just declare struct pg_tm tm; and pass the &tm same as what GetSQLCurrentDate does? 2. Shouldn't the comment "/* This is just a convenience wrapper for GetCurrentTimeUsec */" be in the function comment for GetCurrentDateTime, instead of in the function body? ~ I added a new commitfest entry for this proposal. https://commitfest.postgresql.org/30/2746/ Is there anything else I should be doing to help get this committed? IIUC it seems ready as-is. Kind Regards, Peter Smith Fujitsu Australia
