[PATCH] robustify hipe_bifs:get_hrvtime/0

Mikael Pettersson mikpe@REDACTED
Thu Aug 26 00:25:23 CEST 2010


The HiPE runtime system has a hipe_bifs:get_hrvtime/0 BIF which
mimics the non-standard gethrvtime() C API.  It's possible to
configure the implementation to use the "perfctr" Linux kernel
extension for performance-monitoring counters, in which case
get_hrvtime has very high precision and low overhead.  Otherwise
it uses the same code as runtime(statistics).

This patch changes the get_hrvtime implementation to do a runtime
check to see if perfctr is available, and to use the fallback code
rather than returning a dummy value if perfctr is unavailable,
which is common.

The current dummy value return is a bug.  It messes up the API
and either breaks callers (they get badarg when trying to compute
on the value) or forces them to implement checks and fallbacks
themselves.  Timing code in HiPE's test suites and benchmarks
is known to be affected.

/Mikael

--- otp_src_R14A/erts/emulator/hipe/hipe_bif1.c.~1~	2009-03-12 13:16:10.000000000 +0100
+++ otp_src_R14A/erts/emulator/hipe/hipe_bif1.c	2010-08-25 23:04:39.000000000 +0200
@@ -876,22 +876,44 @@ BIF_RETTYPE hipe_bifs_misc_timer_clear_0
  * + The fallback, which is the same as {X,_} = runtime(statistics).
  */
 
+static double fallback_get_hrvtime(void)
+{
+    unsigned long ms_user;
+
+    elapsed_time_both(&ms_user, NULL, NULL, NULL);
+    return (double)ms_user;
+}
+
 #if USE_PERFCTR
 
 #include "hipe_perfctr.h"
-static int hrvtime_is_open;
-#define hrvtime_is_started()	hrvtime_is_open
+static int hrvtime_started;	/* 0: closed, +1: perfctr, -1: fallback */
+#define hrvtime_is_started()	(hrvtime_started != 0)
 
 static void start_hrvtime(void)
 {
     if (hipe_perfctr_hrvtime_open() >= 0)
-	hrvtime_is_open = 1;
+	hrvtime_started = 1;
+    else
+	hrvtime_started = -1;
+}
+
+static void stop_hrvtime(void)
+{
+    if (hrvtime_started > 0)
+	hipe_perfctr_hrvtime_close();
+    hrvtime_started = 0;
 }
 
-#define get_hrvtime()		hipe_perfctr_hrvtime_get()
-#define stop_hrvtime()		hipe_perfctr_hrvtime_close()
+static double get_hrvtime(void)
+{
+    if (hrvtime_started > 0)
+	return hipe_perfctr_hrvtime_get();
+    else
+	return fallback_get_hrvtime();
+}
 
-#else
+#else	/* !USE_PERFCTR */
 
 /*
  * Fallback, if nothing better exists.
@@ -902,15 +924,9 @@ static void start_hrvtime(void)
 #define hrvtime_is_started()	1
 #define start_hrvtime()		do{}while(0)
 #define stop_hrvtime()		do{}while(0)
+#define get_hrvtime()		fallback_get_hrvtime()
 
-static double get_hrvtime(void)
-{
-    unsigned long ms_user;
-    elapsed_time_both(&ms_user, NULL, NULL, NULL);
-    return (double)ms_user;
-}
-
-#endif	/* hrvtime support */
+#endif	/* !USE_PERFCTR */
 
 BIF_RETTYPE hipe_bifs_get_hrvtime_0(BIF_ALIST_0)
 {
@@ -918,11 +934,8 @@ BIF_RETTYPE hipe_bifs_get_hrvtime_0(BIF_
     Eterm res;
     FloatDef f;
 
-    if (!hrvtime_is_started()) {
+    if (!hrvtime_is_started())
 	start_hrvtime();
-	if (!hrvtime_is_started())
-	    BIF_RET(NIL); /* arity 0 BIFs may not fail */
-    }
     f.fd = get_hrvtime();
     hp = HAlloc(BIF_P, FLOAT_SIZE_OBJECT);
     res = make_float(hp);


More information about the erlang-patches mailing list