Add high resolution time support (nanoseconds) to Time::HiRes via a new method: hrtime()#24471
Add high resolution time support (nanoseconds) to Time::HiRes via a new method: hrtime()#24471scottchiefbaker wants to merge 4 commits into
Time::HiRes via a new method: hrtime()#24471Conversation
|
For posterity I'm writing down the method I use currently: sub micros {
require Time::HiRes;
my @p = Time::HiRes::gettimeofday();
my $ret = ($p[0] * 1000000) + $p[1];
return $ret;
}This gets us pretty accurate timing, but we can do a even better. |
|
Rather than have it do a multiplication, would it be better to have a function that simply returns the sec and nanosec components in the way gettimeofday does, so no more nanoseconds are spent than necessary? |
|
@Grinnz I did consider that as an option. However, every time I need nanoseconds, I need it as an integer. I'd rather do the math in XS than in Perl. We could add an option to |
|
I'm failing to think of a use case where you care about the floating point precision of Time::HiRes::time, but not about doing extra work. (For example if you're using this for comparison, you can compare seconds and then only compare nanoseconds if those are equal.) Whatever the use cases, this should provide the building blocks for the most useful data. |
|
Secondary question to that regard: does it need to be limited to CLOCK_MONOTONIC? That restricts its usefulness to timing comparisons. Could it take a clock option like clock_gettime? |
It's not happening in CI, which tests There is one failure though: which is easily correctable.
For a single return value hrtime() can use Integer multiplication is pretty quick (at the C level ) on modern hardware, perl's arithmetic tends to be slower due to all the tests done to preserve integers where possible.. As to the floating point result, I did a simple C test, the floating point result for CLOCK_REALTIME does drop some precision: I don't think this is a bad idea, but:
|
I would object to this as it would cause the classic vulnerability when stuck in a hash constructor. |
If it's doing a Also, it's really only a "vulnerability" if the badguy gets flow control. This would just be a regular bug. |
We're not talking about gettimeofday, it can have whatever API is appropriate.
Tell that to bugzilla: https://www.cve.org/CVERecord?id=CVE-2014-1572 |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
It causes a switch of keys and values in the list, so the unintended attack vectors could take any number of natures. |
Returns (seconds, nanoseconds) if passed
|
I modified the function to take an optional boolean param: my $ns = hrtime();
my ($sec, $ns) = hrtime(1);This mimics the behavior of the PHP version. I did not implement For the purposes of "I need nanoseconds for timing some code" we probably want to stick with |
|
That presumes that is the only use case for this proposed function. Which is fine, but the purpose of making it configurable would be to make it useful for other use cases. |
|
I'd prefer |
|
The sanity test failure is because Time::HiRes needs a version bump, BTW |
|
If a shorter monotonic-time API is wanted as well, consider adding |
|
Looking for prior art, darwin systems have a C function I like the names |
|
I find |
|
@chansen-sfr Right, that's why normally when functions are added the authors will include a bit of descriptive text along with it to explain how it works, what it does and what you pass in, what it returns and which units the apply to the returned value. it returns the number of nanoseconds... that's the |
This is somewhat unrelated to the issue being discussed. POSIX explicitly defines |
|
Functionality available on CPAN via Time::Nanos. I'd still like to see it in Core. |
|
It would be good to agree on the API and return value semantics when UV is not 64-bit. Would it make sense to return an NV? At current epoch timestamps, nanoseconds since the epoch are on the order of 2^60. For a double, the spacing between adjacent representable values at that magnitude is approximately: 2^(60 - 52) = 256 As a result, an NV used to represent nanoseconds since the epoch would have a resolution of about 256 ns. |
Or the latest version of POSIX::RT::Clock ;-) |
|
Updated: added Perhaps something along these lines: returns diff --git a/dist/Time-HiRes/HiRes.pm b/dist/Time-HiRes/HiRes.pm
index f1fb463582..0420a182aa 100644
--- a/dist/Time-HiRes/HiRes.pm
+++ b/dist/Time-HiRes/HiRes.pm
@@ -382,6 +382,25 @@ C<CLOCK_MONOTONIC>, which guarantees a monotonically increasing time
value (unlike time() or gettimeofday(), which can be adjusted).
See your system documentation for other possibly supported values.
+=item clock_gettime_ns ( $which )
+
+Like C<clock_gettime()>, but returns the current value of the specified
+POSIX high resolution timer in nanoseconds.
+
+Returns C<undef> on failure.
+
+On systems where Perl's native integer type is 64 bits, the result is
+returned as an integer. On systems where Perl's native integer type is
+32 bits, the result is returned as a floating-point value because the
+number of nanoseconds cannot be represented in a native integer.
+
+At current epoch timestamps, the number of nanoseconds since the epoch
+is on the order of 2^60. Since a double-precision floating-point value
+has 53 bits of precision, values represented this way have a granularity
+of approximately 2^(60 - 52), or about 256 nanoseconds. Therefore,
+C<clock_gettime_ns()> on a 32-bit Perl cannot represent every individual
+nanosecond value.
+
=item clock_getres ( $which )
Return as seconds the resolution of the POSIX high resolution timer
@@ -429,6 +448,21 @@ but not necessarily identical. Note that due to backward
compatibility limitations the returned value may wrap around at about
2147 seconds or at about 36 minutes.
+=item monotonic_ns()
+
+Returns the current value of the POSIX monotonic clock in nanoseconds.
+
+This is equivalent to:
+
+ clock_gettime_ns(CLOCK_MONOTONIC)
+
+The monotonic clock is guaranteed to be monotonically increasing and is
+suitable for measuring elapsed time intervals.
+
+Returns C<undef> on failure.
+
+This function is available only on platforms that support C<CLOCK_MONOTONIC>.
+
=item stat
=item stat FH
diff --git a/dist/Time-HiRes/HiRes.xs b/dist/Time-HiRes/HiRes.xs
index 0c4fde4258..4cebfeb714 100644
--- a/dist/Time-HiRes/HiRes.xs
+++ b/dist/Time-HiRes/HiRes.xs
@@ -896,6 +896,33 @@ nsec_without_unslept(struct timespec *sleepfor,
#endif
+#if defined(TIME_HIRES_CLOCK_GETTIME)
+
+static void _clock_gettime_sv(pTHX_ clockid_t clock_id, bool fractional, SV *dsv) {
+ struct timespec ts;
+ int status;
+
+# ifdef TIME_HIRES_CLOCK_GETTIME_SYSCALL
+ status = syscall(SYS_clock_gettime, clock_id, &ts);
+# else
+ status = clock_gettime(clock_id, &ts);
+# endif
+
+ if (fractional)
+ sv_setnv(dsv, status == 0 ? (NV) ts.tv_sec + (NV) ts.tv_nsec / NV_1E9 : -1);
+ else if (status != 0)
+ sv_setsv(dsv, &PL_sv_undef);
+ else if (UVSIZE == 4)
+ sv_setnv(dsv, (NV) ts.tv_sec * NV_1E9 + (NV) ts.tv_nsec);
+ else
+ sv_setuv(dsv, (UV) ts.tv_sec * (UV) IV_1E9 + (UV) ts.tv_nsec);
+}
+
+#define clock_gettime_sv(clock_id, fractional, dsv) \
+ _clock_gettime_sv(aTHX_ clock_id, fractional, dsv)
+
+#endif
+
/* In case Perl and/or Devel::PPPort are too old, minimally emulate
* IS_SAFE_PATHNAME() (which looks for zero bytes in the pathname). */
#ifndef IS_SAFE_PATHNAME
@@ -1378,22 +1405,19 @@ utime(accessed, modified, ...)
#if defined(TIME_HIRES_CLOCK_GETTIME)
-NV
+void
clock_gettime(clock_id = CLOCK_REALTIME)
clockid_t clock_id
+ ALIAS:
+ Time::HiRes::clock_gettime = 0
+ Time::HiRes::clock_gettime_ns = 1
PREINIT:
- struct timespec ts;
- int status = -1;
- CODE:
-# ifdef TIME_HIRES_CLOCK_GETTIME_SYSCALL
- status = syscall(SYS_clock_gettime, clock_id, &ts);
-# else
- status = clock_gettime(clock_id, &ts);
-# endif
- RETVAL = status == 0 ? ts.tv_sec + (NV) ts.tv_nsec / NV_1E9 : -1;
-
- OUTPUT:
- RETVAL
+ dXSTARG;
+ bool fractional = ix == 0;
+ PPCODE:
+ clock_gettime_sv(clock_id, fractional, TARG);
+ ST(0) = TARG;
+ XSRETURN(1);
#else /* if defined(TIME_HIRES_CLOCK_GETTIME) */
@@ -1507,6 +1531,26 @@ clock()
#endif /* #if defined(TIME_HIRES_CLOCK) && defined(CLOCKS_PER_SEC) */
+#if defined(TIME_HIRES_CLOCK_GETTIME) && defined(CLOCK_MONOTONIC)
+
+void
+monotonic_ns()
+ PREINIT:
+ dXSTARG;
+ PPCODE:
+ clock_gettime_sv(CLOCK_MONOTONIC, FALSE, TARG);
+ ST(0) = TARG;
+ XSRETURN(1);
+
+#else
+
+void
+monotonic_ns()
+ PPCODE:
+ croak("Time::HiRes::monotonic_ns(): unimplemented in this platform");
+
+#endif /* #if defined(TIME_HIRES_CLOCK_GETTIME) && defined(CLOCK_MONOTONIC) */
+
void
stat(...)
PROTOTYPE: ;$ |
|
@scottchiefbaker would this ^^^ API work for you? |
|
Yeah I think this is fine. I just need nanoseconds as an integer, the rest is all gravy. |
|
|
||
| void | ||
| hrtime(as_array = 0) | ||
| bool as_array |
There was a problem hiding this comment.
This argument is illogical in the Perl 5 language and shows zero knowledge of Perl coding.
Either call PP wantarray or C GIMME_V to learn this information the PP Grammar correct way.
| status = clock_gettime(CLOCK_MONOTONIC, &ts); | ||
| # endif | ||
| if (status != 0) | ||
| XSRETURN_EMPTY; |
There was a problem hiding this comment.
XSRETURN_EMPTY should not be used in a PPCODE: block. XSRETURN_FOOs are intended for CODE: blocks where local nor global SP has never been rewound to zero, or the validity/non-SEGV Ness of local var SP is unknown, such as in EU::PXS codegen blocks that can not determine if call_sv() indirectly ever got executed in the body of a XSUB thus causing a possible Perl stack realloc grow event, and the end dev was sloppy or careless in writing SPAGAIN in the correct locations. PPCODE: puts a XSprePuSH: in the codegen at the top, rewinding local SP to 0. "PUTBACK; return;" is all that is needed to return an empty list in a PPCODE block.
| bool as_array | ||
| PPCODE: | ||
| PERL_UNUSED_ARG(as_array); | ||
| croak("Time::HiRes::hrtime(): unimplemented in this platform"); |
There was a problem hiding this comment.
croak("Time::HiRes::hrtime(): unimplemented in this platform");
Replace with "%s" and Split off string "Time::HiRes::hrtime" into arg 1 of croak(), nobody needs ASCII string "Time::HiRes::hrtime" to appear twice in the .so/.dll file.
| PUSHs(sv_2mortal(newSViv(ts.tv_sec))); | ||
| PUSHs(sv_2mortal(newSViv(ts.tv_nsec))); | ||
| } else { | ||
| EXTEND(sp, 1); |
There was a problem hiding this comment.
Eliminate EXTEND 1, it's not needed, all XSUBs are guaranteed 1 slot on PL_stack_sp bc pp_entersub removes 1 SV* from the stack to invoke your CV*. The alternative is to do EXTEND 2 on all paths, the Perl stack is incapable of shrinking for the life of the process. If 8 bytes of malloc() space is the difference between the kernel OOM killing Perl and a successful app run, there r much bigger problems going on. It's not worth the code space or CPU wall Time to bother branching between bounds check of 1 and 2. just do 2 always, or check for 2 on the list return path and no check on scalar return path.
| } else { | ||
| EXTEND(sp, 1); | ||
| PUSHs(sv_2mortal(newSVuv((UV)ts.tv_sec * (UV)IV_1E9 + (UV)ts.tv_nsec))); | ||
| } |
There was a problem hiding this comment.
What if UV is size 32bits such as on i386 Perl? You might want type U64 math here or throw a C syntax error. Truncate to 32 bits 3 times silently is a bad idea here if all 3 inputs are size 64b. Even Visual C 6.0 can be beaten with a heavy stick to cough up support for types U64 and I64 .
|
I've been pondering best strategies for how to implement this, the ended up with the methodology in Alternately I would use the |
I would like to get fast, integer based, timing from Perl.
Time::HiResreturns a float, and is not ideal for this kind of thing.Using
CLOCK_GETTIMEon Posix systems this PR adds thehrtime()method toTime::HiResto return a UV of nanosecods. Care was taken to make this similar in style to the other hires functions of this module. It will fail gracefully with acroak()error ifCLOCK_GETTIMEis not available.POD documentation has been included with explanations of the epoch, and the counter roll-over.
It has been tested out-of-tree as a standalone module and passes except for one test:
I didn't touch any of this code, so I suspect this is a side-effect of building out of tree? Not sure what this error is.
See also: PHP's hrtime()