Skip to content

Commit

Permalink
viml/profile: revert proftime_T to unsigned type neovim#10521
Browse files Browse the repository at this point in the history
- reltimestr(): Produce negative value by comparing the unsigned
  proftime_T value to INT64_MAX.

neovim#10452 (comment)
1. The interfaces of nearly all platforms return uint64_t. INT64_MAX is
   only half of that.
2. Low-level interfaces like this typically define that there is no
   fixed starting point. The only guarantees are that it's (a)
   monotonically increasing at a rate that (b) matches real time.

ref 06af88c
fix neovim#10452
  • Loading branch information
justinmk committed Jul 16, 2019
1 parent f31f2d0 commit bab24a8
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 34 deletions.
2 changes: 1 addition & 1 deletion src/nvim/eval.c
Expand Up @@ -13840,7 +13840,7 @@ static void f_reltimestr(typval_T *argvars, typval_T *rettv, FunPtr fptr)
rettv->v_type = VAR_STRING;
rettv->vval.v_string = NULL;
if (list2proftime(&argvars[0], &tm) == OK) {
rettv->vval.v_string = (char_u *) xstrdup(profile_msg(tm));
rettv->vval.v_string = (char_u *)xstrdup(profile_msg(tm));
}
}

Expand Down
60 changes: 31 additions & 29 deletions src/nvim/profile.c
Expand Up @@ -24,19 +24,15 @@ static proftime_T prof_wait_time;
/// @return the current time
proftime_T profile_start(void) FUNC_ATTR_WARN_UNUSED_RESULT
{
uint64_t now = os_hrtime();
assert(now <= INT64_MAX);
return (proftime_T)now;
return os_hrtime();
}

/// Computes the time elapsed.
///
/// @return Elapsed time from `tm` until now.
proftime_T profile_end(proftime_T tm) FUNC_ATTR_WARN_UNUSED_RESULT
{
uint64_t now = os_hrtime();
assert(now <= INT64_MAX);
return profile_sub((proftime_T)now, tm);
return profile_sub(os_hrtime(), tm);
}

/// Gets a string representing time `tm`.
Expand All @@ -48,7 +44,8 @@ proftime_T profile_end(proftime_T tm) FUNC_ATTR_WARN_UNUSED_RESULT
const char *profile_msg(proftime_T tm) FUNC_ATTR_WARN_UNUSED_RESULT
{
static char buf[50];
snprintf(buf, sizeof(buf), "%10.6lf", (double)tm / 1000000000.0);
snprintf(buf, sizeof(buf), "%10.6lf",
(double)profile_signed(tm) / 1000000000.0);
return buf;
}

Expand All @@ -65,12 +62,8 @@ proftime_T profile_setlimit(int64_t msec) FUNC_ATTR_WARN_UNUSED_RESULT
return profile_zero();
}
assert(msec <= (INT64_MAX / 1000000LL) - 1);
proftime_T nsec = msec * 1000000LL;
uint64_t now = os_hrtime();
assert(now <= INT64_MAX);
int64_t rv;
STRICT_ADD((proftime_T)now, nsec, &rv, int64_t);
return rv;
proftime_T nsec = (proftime_T)msec * 1000000ULL;
return os_hrtime() + nsec;
}

/// Checks if current time has passed `tm`.
Expand All @@ -83,9 +76,8 @@ bool profile_passed_limit(proftime_T tm) FUNC_ATTR_WARN_UNUSED_RESULT
// timer was not set
return false;
}
uint64_t now = os_hrtime();
assert(now <= INT64_MAX);
return profile_cmp((proftime_T)now, tm) < 0;

return profile_cmp(os_hrtime(), tm) < 0;
}

/// Gets the zero time.
Expand Down Expand Up @@ -113,19 +105,20 @@ proftime_T profile_divide(proftime_T tm, int count) FUNC_ATTR_CONST
/// @return `tm1` + `tm2`
proftime_T profile_add(proftime_T tm1, proftime_T tm2) FUNC_ATTR_CONST
{
int64_t rv;
STRICT_ADD(tm1, tm2, &rv, int64_t);
return rv;
return tm1 + tm2;
}

/// Subtracts time `tm2` from `tm1` (may be negative).
/// Subtracts time `tm2` from `tm1`.
///
/// Unsigned overflow (wraparound) occurs if `tm2` is greater than `tm1`.
/// Use `profile_signed()` to get the signed integer value.
///
/// @see profile_signed
///
/// @return `tm1` - `tm2`
proftime_T profile_sub(proftime_T tm1, proftime_T tm2) FUNC_ATTR_CONST
{
int64_t rv;
STRICT_SUB(tm1, tm2, &rv, int64_t);
return rv;
return tm1 - tm2;
}

/// Adds the `self` time from the total time and the `children` time.
Expand Down Expand Up @@ -176,22 +169,31 @@ bool profile_equal(proftime_T tm1, proftime_T tm2) FUNC_ATTR_CONST
return tm1 == tm2;
}

/// Calculates the sign of a 64-bit integer.
/// Converts a proftime_T value `tm` to a signed integer.
///
/// @return -1, 0, or +1
static inline int sgn64(int64_t x) FUNC_ATTR_CONST
/// @return signed representation of the given time value
int64_t profile_signed(proftime_T tm)
FUNC_ATTR_CONST
{
return (int) ((x > 0) - (x < 0));
// (tm > INT64_MAX) is >=150 years, so we can assume it was produced by
// arithmetic of two proftime_T values. For human-readable representation
// (and Vim-compat) we want the difference after unsigned wraparound. #10452
return (tm <= INT64_MAX) ? (int64_t)tm : -(int64_t)(UINT64_MAX - tm);
}

/// Compares profiling times.
///
/// Times `tm1` and `tm2` must be less than 150 years apart.
///
/// @return <0, 0 or >0 if `tm2` < `tm1`, `tm2` == `tm1` or `tm2` > `tm1`
/// @return <0: `tm2` < `tm1`
/// 0: `tm2` == `tm1`
/// >0: `tm2` > `tm1`
int profile_cmp(proftime_T tm1, proftime_T tm2) FUNC_ATTR_CONST
{
return sgn64((int64_t)(tm2 - tm1));
if (tm1 == tm2) {
return 0;
}
return profile_signed(tm2 - tm1) < 0 ? -1 : 1;
}

/// globals for use in the startuptime related functionality (time_*).
Expand Down
2 changes: 1 addition & 1 deletion src/nvim/profile.h
Expand Up @@ -4,7 +4,7 @@
#include <stdint.h>
#include <time.h>

typedef int64_t proftime_T;
typedef uint64_t proftime_T;

#define TIME_MSG(s) do { \
if (time_fd != NULL) time_msg(s, NULL); \
Expand Down
10 changes: 7 additions & 3 deletions test/functional/eval/reltime_spec.lua
Expand Up @@ -38,9 +38,13 @@ describe('reltimestr(), reltimefloat()', function()
local older_time = reltime()
command('sleep 1m')
local newer_time = reltime()
-- Should be something like -0.002123.

-- Start/end swapped: should be something like -0.002123.
local rv = tonumber(reltimestr(reltime(newer_time, older_time)))
ok(rv < 0)
ok(rv > -10)
ok(rv < 0 and rv > -10)

-- Not swapped: should be something like 0.002123.
rv = tonumber(reltimestr(reltime(older_time, newer_time)))
ok(rv > 0 and rv < 10)
end)
end)

0 comments on commit bab24a8

Please sign in to comment.