From fea8c8e40611581d85ab5f96ea5a11e348c02247 Mon Sep 17 00:00:00 2001
From: Mons Anderson <mons@cpan.org>
Date: Wed, 14 Apr 2021 01:17:44 +0300
Subject: [PATCH] clock: return signed ints from time64 functions
Changed return value type in Lua for the following functions from
`uint64_t` to `int64_t`:
* clock.time64
* clock.realtime64
* clock.monotonic64
* clock.process64
* clock.thread64
* fiber.time64
* fiber.clock64
Changed return value type in Module API for the following functions from
`uint64_t` to `int64_t`:
* fiber_time64
* fiber_clock64
Also enhanced clock.test.lua tap test with more testcases
Due to unsigned values returned from time64 family function time
calculations may lead to undesired value overflows. Having two 64-bit
timestamps I cannot just subtract them without prior comparison. It is
used often, for example when calculating time to a deadline (in queues
or for expiration).
Let's measure time, that pass between two sequential clock.time calls:
```lua
print(clock.time() - clock.time()) -- -9.5367431640625e-07
```
It's ok, with double type time return, but what about 64-bit integers?
```
print(clock.time64() - clock.time64()) -- 18446744073709550616
```
It returns weird integer overflow instead of expected `-1000`
This is counterintuitive and gets conflicted with every time functions
from system headers (which are always signed).
If we consider some hypothetical code, that waits some time for a
deadline, then instead of exiting loop, this code will go to "infinite"
(actually 585 years) wait in the case when the deadline was passed.
```lua
do
local ts = clock.time64() + 1e9 -- some_timestamp_in_future, ex: now+1s
-- ...
local remaining = clock.time64() - ts
-- ...
if remaining > 0 then
fiber.sleep( tonumber(remaining/1e9) )
end
print "1 second passed"
end
```
This is hard to notice (in real tests deadlines are always in the
future), but in a real environment and under high load such cases occur.
1. Since Lua functions will return ffi.typeof `ctype<int64_t>` instead
of old `ctype<uint64_t>` any code, that uses _strict_ ffi type check
will fail. But I hardly believe such code ever exists.
2. Low-level Module API also changed, and code, which compiles with
`-Werror` may also fail. This could be an issue and in such a case we
may remove the update of module API change. Still, I consider this
change reasonable, since all C-level API uses signed time values.
@TarantoolBot document
Title: Return type of time64 functions is changed
These C functions in the public API used to return `uint64_t`, now they
return `int64_t`: `clock_realtime64`, `clock_monotonic64`,
`clock_process64`, `clock_thread64`, `fiber_time64()`,
`fiber_clock64()`.
These Lua functions used to return cdata of type `uint64_t`, now they
return cdata of type `int64_t`: `fiber.time64()`, `fiber.clock64()`.
---
.../changed-signedness-of-time64-functions.md | 10 +++
src/lib/core/clock.c | 22 +++---
src/lib/core/clock.h | 70 ++++++++++++++++---
src/lib/core/fiber.c | 8 +--
src/lib/core/fiber.h | 4 +-
src/lua/clock.lua | 8 +--
src/lua/fiber.lua | 4 +-
test/app-tap/clock.test.lua | 60 +++++++++++++++-
8 files changed, 153 insertions(+), 33 deletions(-)
create mode 100644 changelogs/unreleased/changed-signedness-of-time64-functions.md
diff --git a/changelogs/unreleased/changed-signedness-of-time64-functions.md b/changelogs/unreleased/changed-signedness-of-time64-functions.md
new file mode 100644
index 0000000000..3e500994c0
--- /dev/null
+++ b/changelogs/unreleased/changed-signedness-of-time64-functions.md
@@ -0,0 +1,10 @@
+## bugfix/core
+
+* **[Breaking change]** Return value signedness of 64-bit time functions in
+ `clock` and `fiber` was changed from `uint64_t` to `int64_t` both in Lua
+ and C (gh-5989).
+
+----
+
+Breaking change: lua: return value type for all time64 functions changed from
+`uint64_t` to `int64_t`.
diff --git a/src/lib/core/clock.c b/src/lib/core/clock.c
index ff30584857..ccf35858c8 100644
--- a/src/lib/core/clock.c
+++ b/src/lib/core/clock.c
@@ -70,41 +70,43 @@ clock_thread(void)
#endif
}
-uint64_t
+int64_t
clock_realtime64(void)
{
struct timespec ts;
clock_gettime(CLOCK_REALTIME, &ts);
- return ((uint64_t)ts.tv_sec) * 1000000000 + ts.tv_nsec;
+ return ((int64_t)ts.tv_sec) * 1000000000 + ts.tv_nsec;
}
-uint64_t
+
+int64_t
clock_monotonic64(void)
{
struct timespec ts;
clock_gettime(CLOCK_MONOTONIC, &ts);
- return ((uint64_t)ts.tv_sec) * 1000000000 + ts.tv_nsec;
+ return ((int64_t)ts.tv_sec) * 1000000000 + ts.tv_nsec;
}
-uint64_t
+
+int64_t
clock_process64(void)
{
#if defined(CLOCK_PROCESS_CPUTIME_ID)
struct timespec ts;
clock_gettime(CLOCK_PROCESS_CPUTIME_ID, &ts);
- return ((uint64_t)ts.tv_sec) * 1000000000 + ts.tv_nsec;
+ return ((int64_t)ts.tv_sec) * 1000000000 + ts.tv_nsec;
#else
- return (uint64_t) clock() * 1000000000 / CLOCKS_PER_SEC;
+ return (int64_t)clock() * 1000000000 / CLOCKS_PER_SEC;
#endif
}
-uint64_t
+int64_t
clock_thread64(void)
{
#if defined(CLOCK_THREAD_CPUTIME_ID)
struct timespec ts;
clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts);
- return ((uint64_t)ts.tv_sec) * 1000000000 + ts.tv_nsec;
+ return ((int64_t)ts.tv_sec) * 1000000000 + ts.tv_nsec;
#else
- return (uint64_t) clock() * 1000000000 / CLOCKS_PER_SEC;
+ return (int64_t)clock() * 1000000000 / CLOCKS_PER_SEC;
#endif
}
diff --git a/src/lib/core/clock.h b/src/lib/core/clock.h
index e31b8599ad..75376b5614 100644
--- a/src/lib/core/clock.h
+++ b/src/lib/core/clock.h
@@ -40,15 +40,67 @@ extern "C" {
/** \cond public */
-double clock_realtime(void);
-double clock_monotonic(void);
-double clock_process(void);
-double clock_thread(void);
-
-uint64_t clock_realtime64(void);
-uint64_t clock_monotonic64(void);
-uint64_t clock_process64(void);
-uint64_t clock_thread64(void);
+/**
+ * A settable system-wide clock that measures real (i.e.,
+ * wall-clock) time.
+ *
+ * See clock_gettime(2), CLOCK_REALTIME.
+ */
+double
+clock_realtime(void);
+
+/**
+ * A nonsettable system-wide clock that represents monotonic time.
+ *
+ * See clock_gettime(2), CLOCK_MONOTONIC.
+ */
+double
+clock_monotonic(void);
+
+/**
+ * A clock that measures CPU time consumed by this process (by all
+ * threads in the process).
+ *
+ * See clock_gettime(2), CLOCK_PROCESS_CPUTIME_ID.
+ */
+double
+clock_process(void);
+
+/**
+ * A clock that measures CPU time consumed by this thread.
+ *
+ * See clock_gettime(2), CLOCK_THREAD_CPUTIME_ID.
+ */
+double
+clock_thread(void);
+
+/**
+ * Same as clock_realtime(), but returns the time as 64 bit
+ * signed integer.
+ */
+int64_t
+clock_realtime64(void);
+
+/**
+ * Same as clock_monotonic(), but returns the time as 64 bit
+ * signed integer.
+ */
+int64_t
+clock_monotonic64(void);
+
+/**
+ * Same as clock_process(), but returns the time as 64 bit
+ * signed integer.
+ */
+int64_t
+clock_process64(void);
+
+/**
+ * Same as clock_thread(), but returns the time as 64 bit
+ * signed integer.
+ */
+int64_t
+clock_thread64(void);
/** \endcond public */
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 147ff49b49..19e73d7036 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -592,10 +592,10 @@ fiber_time(void)
return ev_now(loop());
}
-uint64_t
+int64_t
fiber_time64(void)
{
- return (uint64_t) ( ev_now(loop()) * 1000000 + 0.5 );
+ return (int64_t)(ev_now(loop()) * 1000000 + 0.5);
}
double
@@ -604,10 +604,10 @@ fiber_clock(void)
return ev_monotonic_now(loop());
}
-uint64_t
+int64_t
fiber_clock64(void)
{
- return (uint64_t) ( ev_monotonic_now(loop()) * 1000000 + 0.5 );
+ return (int64_t)(ev_monotonic_now(loop()) * 1000000 + 0.5);
}
/**
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index 5d9fd6d6da..4ed5c1d81e 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -391,7 +391,7 @@ fiber_time(void);
* Report loop begin time as 64-bit int.
* Uses real time clock.
*/
-API_EXPORT uint64_t
+API_EXPORT int64_t
fiber_time64(void);
/**
@@ -405,7 +405,7 @@ fiber_clock(void);
* Report loop begin time as 64-bit int.
* Uses monotonic clock.
*/
-API_EXPORT uint64_t
+API_EXPORT int64_t
fiber_clock64(void);
/**
diff --git a/src/lua/clock.lua b/src/lua/clock.lua
index fee43ccde7..738f3844fe 100644
--- a/src/lua/clock.lua
+++ b/src/lua/clock.lua
@@ -7,10 +7,10 @@ ffi.cdef[[
double clock_monotonic(void);
double clock_process(void);
double clock_thread(void);
- uint64_t clock_realtime64(void);
- uint64_t clock_monotonic64(void);
- uint64_t clock_process64(void);
- uint64_t clock_thread64(void);
+ int64_t clock_realtime64(void);
+ int64_t clock_monotonic64(void);
+ int64_t clock_process64(void);
+ int64_t clock_thread64(void);
]]
local C = ffi.C
diff --git a/src/lua/fiber.lua b/src/lua/fiber.lua
index b141177149..a788aa37d3 100644
--- a/src/lua/fiber.lua
+++ b/src/lua/fiber.lua
@@ -5,11 +5,11 @@ local ffi = require('ffi')
ffi.cdef[[
double
fiber_time(void);
-uint64_t
+int64_t
fiber_time64(void);
double
fiber_clock(void);
-uint64_t
+int64_t
fiber_clock64(void);
]]
local C = ffi.C
diff --git a/test/app-tap/clock.test.lua b/test/app-tap/clock.test.lua
index d1ea4f0a8e..5f25d77bbc 100755
--- a/test/app-tap/clock.test.lua
+++ b/test/app-tap/clock.test.lua
@@ -1,16 +1,72 @@
#!/usr/bin/env tarantool
local clock = require("clock")
-local test = require("tap").test("csv")
-test:plan(10)
+local fiber = require("fiber")
+local test = require("tap").test("clock")
+
+test:plan(36)
+
+test:ok(clock.time() > 0, "time")
test:ok(clock.realtime() > 0, "realtime")
test:ok(clock.thread() > 0, "thread")
test:ok(clock.monotonic() > 0, "monotonic")
test:ok(clock.proc() > 0, "proc")
+test:ok(fiber.time() > 0, "fiber.time")
+test:ok(fiber.clock() > 0, "fiber.clock")
+test:ok(clock.time64() > 0, "time64")
test:ok(clock.realtime64() > 0, "realtime64")
test:ok(clock.thread64() > 0, "thread64")
test:ok(clock.monotonic64() > 0, "monotonic64")
test:ok(clock.proc64() > 0, "proc64")
+test:ok(fiber.time64() > 0, "fiber.time64")
+test:ok(fiber.clock64() > 0, "fiber.clock64")
test:ok(clock.monotonic() <= clock.monotonic(), "time is monotonic")
+test:ok(clock.monotonic64() <= clock.monotonic64(), "time is monotonic")
test:ok(math.abs(clock.realtime() - os.time()) < 2, "clock.realtime ~ os.time")
+
+test:ok(fiber.time() == fiber.time(), "fiber.time is cached")
+test:ok(fiber.time64() == fiber.time64(), "fiber.time64 is cached")
+
+test:ok(fiber.clock() == fiber.clock(), "fiber.clock is cached")
+test:ok(fiber.clock64() == fiber.clock64(), "fiber.clock64 is cached")
+test:ok(fiber.clock() < (fiber.yield() or 0) + fiber.clock(),
+ "fiber.clock is growing after yield")
+test:ok(fiber.clock64() < (fiber.yield() or 0) + fiber.clock64(),
+ "fiber.clock64 is growing after yield")
+
+test:ok(math.abs(fiber.time() - tonumber(fiber.time64())/1e6) < 1,
+ "fiber.time64 is in microseconds")
+test:ok(math.abs(fiber.clock() - tonumber(fiber.clock64())/1e6) < 1,
+ "fiber.clock64 is in microseconds")
+
+test:ok(math.abs(clock.time() - tonumber(clock.time64())/1e9) < 1,
+ "clock.time64 is in nanoseconds")
+test:ok(math.abs(clock.realtime() - tonumber(clock.realtime64())/1e9) < 1,
+ "clock.realtime64 is in nanoseconds")
+test:ok(math.abs(clock.thread() - tonumber(clock.thread64())/1e9) < 1,
+ "clock.thread64 is in nanoseconds")
+test:ok(math.abs(clock.proc() - tonumber(clock.proc64())/1e9) < 1,
+ "clock.proc64 is in nanoseconds")
+
+local function subtract_future(func)
+ local ts1 = func()
+ fiber.sleep(0.001)
+ return ts1 - func()
+end
+test:ok(subtract_future(clock.time64) < 0,
+ "clock.time64() can be subtracted")
+test:ok(subtract_future(clock.realtime64) < 0,
+ "clock.realtime64() can be subtracted")
+test:ok(subtract_future(clock.thread64) < 0,
+ "clock.thread64() can be subtracted")
+test:ok(subtract_future(clock.monotonic64) < 0,
+ "clock.monotonic64() can be subtracted")
+test:ok(subtract_future(clock.proc64) < 0,
+ "clock.proc64() can be subtracted")
+test:ok(subtract_future(fiber.time64) < 0,
+ "fiber.time64 can be subtracted")
+test:ok(subtract_future(fiber.clock64) < 0,
+ "fiber.clock64 can be subtracted")
+
+os.exit(test:check() and 0 or 1)
--
GitLab