impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5357: Fix unixtime to UTC TimestampValue perf
Date Tue, 06 Jun 2017 03:48:28 GMT
Hello Marcel Kornacker,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/7082

to look at the new patch set (#2).

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
---
M be/src/runtime/timestamp-test.cc
M be/src/runtime/timestamp-value.cc
2 files changed, 66 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/7082/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7082
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9d611d21310c7b4f93d8e0bc845eb85125abcac9
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Attila Jeges <attilaj@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>

Mime
View raw message