Impala Public Jenkins has submitted this change and it was merged. Change subject: IMPALA-5357: Fix unixtime to UTC TimestampValue perf ......................................................................
IMPALA-5357: Fix unixtime to UTC TimestampValue perf Fixes the severe perf issue when calling gmtime_r to convert a unix time because that libc function takes a global lock. Instead, use boost ptime::from_time_t when possible. Unfortunately only a range of input times are supported without overflowing the underlying time_duration struct (dates between 1677-2262), so those dates outside that range are handled with the slow path calling into gmtime_r. This requires using a patched build of boost which includes an upstream fix for the time_duration class, see: https://github.com/cloudera/native-toolchain/commit/6e726b4b8164d53814f75b78a681fcf6df4a887a Testing: * Ran an exhaustive test run successfully. * Wrote a test program to validate that all time_t values between 1677 and 2262 are converted to the same ptime when using the new code path and the old path. Doing so required running all night, so I'm not going to attempt to add this test. I think a single validation is acceptable. Perf impact: Microbenchmark of the new path (conversion using boost) and the old path (conversion using libc gmtime_r) resulted in the expected speedup from not using a global lock. Machine Info: Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz Function 10%ile 50%ile 90%ile 10%ile 50%ile 90%ile iters/ms iters/ms iters/ms (relative) (relative) (relative) --------------------------------------------------------------------- libc-1 0.192 0.196 0.2 1X 1X 1X boost-1 0.333 0.34 0.34 1.73X 1.73X 1.7X libc-2 0.112 0.115 0.116 0.584X 0.586X 0.581X boost-2 0.627 0.654 0.667 3.26X 3.33X 3.33X libc-4 0.042 0.0478 0.0515 0.218X 0.244X 0.258X boost-4 0.863 1.27 1.3 4.49X 6.5X 6.5X libc-8 0.0326 0.0328 0.0329 0.169X 0.167X 0.164X boost-8 0.741 0.902 1.1 3.85X 4.6X 5.5X Change-Id: I9d611d21310c7b4f93d8e0bc845eb85125abcac9 Reviewed-on: http://gerrit.cloudera.org:8080/7082 Reviewed-by: Matthew Jacobs <m...@cloudera.com> Tested-by: Impala Public Jenkins --- M be/src/runtime/timestamp-test.cc M be/src/runtime/timestamp-value.cc 2 files changed, 66 insertions(+), 13 deletions(-) Approvals: Impala Public Jenkins: Verified Matthew Jacobs: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/7082 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I9d611d21310c7b4f93d8e0bc845eb85125abcac9 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com>
