impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tarmstr...@apache.org
Subject [2/3] incubator-impala git commit: IMPALA-4631: don't use floating point operations for time unit conversions
Date Thu, 15 Dec 2016 23:03:32 GMT
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 <dhecht@cloudera.com>
Tested-by: Jim Apple <jbapple-impala@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 <dhecht@cloudera.com>
Authored: Thu Dec 8 15:29:47 2016 -0800
Committer: Dan Hecht <dhecht@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.


Mime
View raw message