You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2016/12/15 23:03:32 UTC
[2/3] incubator-impala git commit: IMPALA-4631: don't use floating
point operations for time unit conversions
IMPALA-4631: don't use floating point operations for time unit conversions
This was leading to the PlanFragmentExecutor::Close() DCHECK because
with floating point we can have c * a + c * b > c * (a + b). Also note
this is much more likely to happen when using the MONOTONIC_COARSE since
that will result in the nested scoped timers ending up starting/stopping
at exactly the same time. Additionally, the new code is faster.
Change-Id: I7237f579b201f5bd3930f66e9c2c8d700c37ffeb
Reviewed-on: http://gerrit.cloudera.org:8080/5434
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Jim Apple <jb...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/54194af6
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/54194af6
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/54194af6
Branch: refs/heads/master
Commit: 54194af6ef08048a1ae367f29350228dafd8f2aa
Parents: d6eb1b1
Author: Dan Hecht <dh...@cloudera.com>
Authored: Thu Dec 8 15:29:47 2016 -0800
Committer: Dan Hecht <dh...@cloudera.com>
Committed: Thu Dec 15 22:37:47 2016 +0000
----------------------------------------------------------------------
be/src/gutil/walltime.cc | 12 ------------
be/src/gutil/walltime.h | 20 ++++++++++++--------
be/src/util/stopwatch.h | 2 +-
be/src/util/time.h | 6 +++---
4 files changed, 16 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/54194af6/be/src/gutil/walltime.cc
----------------------------------------------------------------------
diff --git a/be/src/gutil/walltime.cc b/be/src/gutil/walltime.cc
index b497500..04d7f4b 100644
--- a/be/src/gutil/walltime.cc
+++ b/be/src/gutil/walltime.cc
@@ -166,18 +166,6 @@ bool WallTime_Parse_Timezone(const char* time_spec,
return true;
}
-WallTime WallTime_Now() {
-#if defined(__APPLE__)
- mach_timespec_t ts;
- walltime_internal::GetCurrentTime(&ts);
- return ts.tv_sec + ts.tv_nsec / static_cast<double>(1e9);
-#else
- timespec ts;
- clock_gettime(CLOCK_REALTIME, &ts);
- return ts.tv_sec + ts.tv_nsec / static_cast<double>(1e9);
-#endif // defined(__APPLE__)
-}
-
void StringAppendStrftime(string* dst,
const char* format,
time_t when,
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/54194af6/be/src/gutil/walltime.h
----------------------------------------------------------------------
diff --git a/be/src/gutil/walltime.h b/be/src/gutil/walltime.h
index 8d31627..2f04ebe 100644
--- a/be/src/gutil/walltime.h
+++ b/be/src/gutil/walltime.h
@@ -30,6 +30,12 @@ using std::string;
#include "common/logging.h"
#include "gutil/integral_types.h"
+#define NANOS_PER_SEC 1000000000ll
+#define NANOS_PER_MICRO 1000ll
+#define MICROS_PER_SEC 1000000ll
+#define MICROS_PER_MILLI 1000ll
+#define MILLIS_PER_SEC 1000ll
+
typedef double WallTime;
// Append result to a supplied string.
@@ -52,9 +58,6 @@ bool WallTime_Parse_Timezone(const char* time_spec,
bool local,
WallTime* result);
-// Return current time in seconds as a WallTime.
-WallTime WallTime_Now();
-
typedef int64 MicrosecondsInt64;
typedef int64 NanosecondsInt64;
@@ -76,7 +79,7 @@ inline void GetCurrentTime(mach_timespec_t* ts) {
inline MicrosecondsInt64 GetCurrentTimeMicros() {
mach_timespec_t ts;
GetCurrentTime(&ts);
- return ts.tv_sec * 1e6 + ts.tv_nsec / 1e3;
+ return ts.tv_sec * MICROS_PER_SEC + ts.tv_nsec / NANOS_PER_MICRO;
}
inline int64_t GetMonoTimeNanos() {
@@ -91,7 +94,7 @@ inline int64_t GetMonoTimeNanos() {
}
inline MicrosecondsInt64 GetMonoTimeMicros() {
- return GetMonoTimeNanos() / 1e3;
+ return GetMonoTimeNanos() / NANOS_PER_MICRO;
}
inline MicrosecondsInt64 GetThreadCpuTimeMicros() {
@@ -117,7 +120,8 @@ inline MicrosecondsInt64 GetThreadCpuTimeMicros() {
return 0;
}
- return thread_info_data.user_time.seconds * 1e6 + thread_info_data.user_time.microseconds;
+ return thread_info_data.user_time.seconds * MICROS_PER_SEC +
+ thread_info_data.user_time.microseconds;
}
#else
@@ -125,13 +129,13 @@ inline MicrosecondsInt64 GetThreadCpuTimeMicros() {
inline MicrosecondsInt64 GetClockTimeMicros(clockid_t clock) {
timespec ts;
clock_gettime(clock, &ts);
- return ts.tv_sec * 1e6 + ts.tv_nsec / 1e3;
+ return ts.tv_sec * MICROS_PER_SEC + ts.tv_nsec / NANOS_PER_MICRO;
}
inline NanosecondsInt64 GetClockTimeNanos(clockid_t clock) {
timespec ts;
clock_gettime(clock, &ts);
- return ts.tv_sec * 1e9 + ts.tv_nsec;
+ return ts.tv_sec * NANOS_PER_SEC + ts.tv_nsec;
}
#endif // defined(__APPLE__)
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/54194af6/be/src/util/stopwatch.h
----------------------------------------------------------------------
diff --git a/be/src/util/stopwatch.h b/be/src/util/stopwatch.h
index 0e73b6a..c1f85aa 100644
--- a/be/src/util/stopwatch.h
+++ b/be/src/util/stopwatch.h
@@ -170,7 +170,7 @@ class MonotonicStopWatch {
// Now() can be called frequently (IMPALA-2407).
timespec ts;
clock_gettime(OsInfo::fast_clock(), &ts);
- return ts.tv_sec * 1e9 + ts.tv_nsec;
+ return ts.tv_sec * NANOS_PER_SEC + ts.tv_nsec;
#endif
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/54194af6/be/src/util/time.h
----------------------------------------------------------------------
diff --git a/be/src/util/time.h b/be/src/util/time.h
index 28a0f66..efe6a3b 100644
--- a/be/src/util/time.h
+++ b/be/src/util/time.h
@@ -39,11 +39,11 @@ inline int64_t MonotonicMicros() { // 63 bits ~= 5K years uptime
}
inline int64_t MonotonicMillis() {
- return GetMonoTimeMicros() / 1e3;
+ return GetMonoTimeMicros() / MICROS_PER_MILLI;
}
inline int64_t MonotonicSeconds() {
- return GetMonoTimeMicros() / 1e6;
+ return GetMonoTimeMicros() / MICROS_PER_SEC;
}
@@ -52,7 +52,7 @@ inline int64_t MonotonicSeconds() {
/// a cluster. For more accurate timings on the local host use the monotonic functions
/// above.
inline int64_t UnixMillis() {
- return GetCurrentTimeMicros() / 1e3;
+ return GetCurrentTimeMicros() / MICROS_PER_MILLI;
}
/// Sleeps the current thread for at least duration_ms milliseconds.