Skip to content
This repository

linux-core.c: return time values in ticks, not seconds #662

Closed
wants to merge 1 commit into from

2 participants

Tim Bradshaw Ben Noordhuis
Tim Bradshaw

(still less the bogus values that were being returned previously)

CPU time fields were being spuriously multiplied by the ticks/second value resulting in insane values on Linux. This patch changes that so they're just returned as raw ticks.

Since node.js offers no way to get the ticks/second value this is still not very useful but it does mean the values actually correspond to what the OS thinks.

In an ideal world I think it would be better to either provide an interface to getting the ticks/second value from the OS, or to return CPU times in seconds. Both of those would be fairly easy changes I think: probably the former is better as it does not change things incompatibly (though I assume no one has being relying on the existing values!)

Tim Bradshaw linux-core.c: return time values in ticks, not seconds (still less th…
…e bogus values that were being returned previously)
036f9ca
Ben Noordhuis

I can't take this patch, it's inconsistent with what libuv does on other platforms.

or to return CPU times in seconds

I think that was the intended goal. (The original code wasn't written by me; it's bug-for-bug adapted from node v0.4.)

Does this patch look acceptable to you?

diff --git a/src/unix/linux/linux-core.c b/src/unix/linux/linux-core.c
index 95df667..102be43 100644
--- a/src/unix/linux/linux-core.c
+++ b/src/unix/linux/linux-core.c
@@ -640,11 +640,11 @@ static void read_times(unsigned int numcpus, uv_cpu_info_t* ci) {
                     &irq))
       abort();

-    ts.user = clock_ticks * user;
-    ts.nice = clock_ticks * nice;
-    ts.sys  = clock_ticks * sys;
-    ts.idle = clock_ticks * idle;
-    ts.irq  = clock_ticks * irq;
+    ts.user = user / clock_ticks;
+    ts.nice = nice / clock_ticks;
+    ts.sys  = sys / clock_ticks;
+    ts.idle = idle / clock_ticks;
+    ts.irq  = irq / clock_ticks;
     ci[num++].cpu_times = ts;
   }
   fclose(fp);
Tim Bradshaw
Tim Bradshaw
Ben Noordhuis

What I think is the right thing to do is return times in seconds, as node provides no way of knowing how long a tick is, so times in ticks are not really useful other than for comparison).

Yes, seconds or possibly milliseconds.

It's not unlikely that the actual code does something different; most of the code comes straight from node v0.4 and was patched together by a small army of volunteers.

A quick look at the Windows implementation suggests in returns the times in milliseconds.

Tim Bradshaw
Ben Noordhuis

Yes, sounds good. The BSD + Darwin implementations will need to be reviewed as well though I believe they already do the right thing. I'll close this PR.

Ben Noordhuis bnoordhuis closed this December 31, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Dec 21, 2012
Tim Bradshaw linux-core.c: return time values in ticks, not seconds (still less th…
…e bogus values that were being returned previously)
036f9ca
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 10 additions and 10 deletions. Show diff stats Hide diff stats

  1. 20  src/unix/linux/linux-core.c
20  src/unix/linux/linux-core.c
@@ -584,7 +584,6 @@ static void read_models(unsigned int numcpus, uv_cpu_info_t* ci) {
584 584
 
585 585
 
586 586
 static void read_times(unsigned int numcpus, uv_cpu_info_t* ci) {
587  
-  unsigned long clock_ticks;
588 587
   struct uv_cpu_times_s ts;
589 588
   unsigned long user;
590 589
   unsigned long nice;
@@ -597,10 +596,6 @@ static void read_times(unsigned int numcpus, uv_cpu_info_t* ci) {
597 596
   char buf[1024];
598 597
   FILE* fp;
599 598
 
600  
-  clock_ticks = sysconf(_SC_CLK_TCK);
601  
-  assert(clock_ticks != (unsigned long) -1);
602  
-  assert(clock_ticks != 0);
603  
-
604 599
   fp = fopen("/proc/stat", "r");
605 600
   if (fp == NULL)
606 601
     return;
@@ -627,6 +622,11 @@ static void read_times(unsigned int numcpus, uv_cpu_info_t* ci) {
627 622
     /* Line contains user, nice, system, idle, iowait, irq, softirq, steal,
628 623
      * guest, guest_nice but we're only interested in the first four + irq.
629 624
      *
  625
+     * The time values are already defined in terms of ticks (not
  626
+     * seconds), and since the node manual defines the corresponding
  627
+     * values in terms of ticks, we just return those.  This is also
  628
+     * what other platforms do.
  629
+     *
630 630
      * Don't use %*s to skip fields or %ll to read straight into the uint64_t
631 631
      * fields, they're not allowed in C89 mode.
632 632
      */
@@ -640,11 +640,11 @@ static void read_times(unsigned int numcpus, uv_cpu_info_t* ci) {
640 640
                     &irq))
641 641
       abort();
642 642
 
643  
-    ts.user = clock_ticks * user;
644  
-    ts.nice = clock_ticks * nice;
645  
-    ts.sys  = clock_ticks * sys;
646  
-    ts.idle = clock_ticks * idle;
647  
-    ts.irq  = clock_ticks * irq;
  643
+    ts.user = user;
  644
+    ts.nice = nice;
  645
+    ts.sys  = sys;
  646
+    ts.idle = idle;
  647
+    ts.irq  = irq;
648 648
     ci[num++].cpu_times = ts;
649 649
   }
650 650
   fclose(fp);
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.