Timer.sleep suspends System.clock

[PureFox48]

trafficstars

I noticed today that Timer.sleep not only suspends the current fiber but it also suspends System.clock.

For example, if I run this script (using the time command on Linux for comparison) I obtain the results shown:

import "timer" for Timer

var start = System.clock
Timer.sleep(2000)
System.print("Time elapsed since program started is %(System.clock-start) seconds.")

/*
    Time elapsed since program started is 8.9e-05 seconds.

    real   0m2.007s
    user   0m0.000s
    sys    0m0.005s
*/

If this is expected behavior, I think it should be documented somewhere. The question is where?

The Timer class is currently undocumented and System.clock is documented in the main repo rather than here.

[ChayimFriedman2]

I think this is an unexpected behavior, since System.clock is meant for benchmarking. But changing it is not easy at all.

About the Timer class undocumented - send a PR! About System.clock in the main repo - it's there because it should be there.

[PureFox48]

I've just realized that the CLI modules are still documented in the main repo as well.

I've prepared PRs to document both the Timer and Scheduler classes and will post them shortly.

[robiniddon]

The system.clock primitive is using time.h's clock() call which is equivalent to

clock_gettime(CLOCK_PROCESS_CPUTIME_ID, &my_timespec)

This only records time spent running on the CPU for the process, not elapsed time (aka real in time's output above).

This feels like a sane thing to want to know, but if looking for the equivalent of Java's System.currentTimeMillis() then we need to change code in the vm/wren_core.c:

diff --git a/deps/wren/src/vm/wren_core.c b/deps/wren/src/vm/wren_core.c
index c1d5fd2..c298c5f 100644
--- a/deps/wren/src/vm/wren_core.c
+++ b/deps/wren/src/vm/wren_core.c
@@ -1123,7 +1123,10 @@ DEF_PRIMITIVE(string_toString)
 
 DEF_PRIMITIVE(system_clock)
 {
-  RETURN_NUM((double)clock() / CLOCKS_PER_SEC);
+  struct timespec tv;
+  clock_gettime(CLOCK_REALTIME, &tv);
+  RETURN_NUM((double)((tv.tv_sec) + (tv.tv_nsec/1000000000L)));
 }

This then yields:

Time elapsed since program started is 2 seconds.

real	0m2.005s
user	0m0.001s
sys	0m0.001s

But as System.clock() exists already this would be a potentially breaking change. Also for benchmarking you would really want to use the MONOTONIC clock as this won't go backwards/forwards due to time of day updates, timezone changes, etc.

[joshgoebel]

According to the documentation:

Returns the number of seconds (including fractional seconds) since the program was started. This is usually used for benchmarking.

Based on that I'd say the current implementation is obviously broken or else the documentation is not correct. For the stated behavior using CLOCK_REALTIME (or equivalent) sounds good (if that is portable).

So the bug here seems to be with System.clock() rather than Timer.sleep(). This issue should probably be moved/copied into the main Wren repository instead?

CC @ruby0x1