Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005434OCamlOCaml generalpublic2011-12-18 14:222012-09-25 20:07
Reporterjjb 
Assigned To 
PrioritynormalSeverityfeatureReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version3.12.1 
Target VersionFixed in Version 
Summary0005434: Unix.times unimplemented/suboptimal on Windows
DescriptionHere is a slightly improved implementation of Unix.times for Windows. It uses the Windows GetProcessTimes call, which has existed since Windows 2000. User and System time is reported in 100-ns units, although actual precision is less (seems to be 1/64 sec). This implementation makes no effort to report information on child processes.

----------


#include <windows.h>
#include <caml/mlvalues.h>
#include <caml/fail.h>


double to_sec(FILETIME ft) {
  ULARGE_INTEGER tmp;

  tmp.u.LowPart = ft.dwLowDateTime;
  tmp.u.HighPart = ft.dwHighDateTime;

  /* convert to seconds:
     GetProcessTimes returns number of 100-nanosecond intervals */
  return tmp.QuadPart / 1e7;
}


value times(value unit) {

  value res;
  FILETIME creation, exit, stime, utime;
  ULARGE_INTEGER tmp;

  if (!(GetProcessTimes(GetCurrentProcess(), &creation, &exit, &stime, &utime)))
    caml_failwith("GetProcessTimes failed");

  res = alloc_small(4 * Double_wosize, Double_array_tag);
  Store_double_field(res, 0, to_sec(utime));
  Store_double_field(res, 1, to_sec(stime));
  Store_double_field(res, 2, 0);
  Store_double_field(res, 3, 0);
  return res;

}
TagsNo tags attached.
Attached Filespatch file icon win32unix_times.patch [^] (2,325 bytes) 2011-12-20 11:45 [Show Content]

- Relationships

-  Notes
(0006378)
ripoche (reporter)
2011-12-18 15:43
edited on: 2011-12-18 15:44

I had something like that in mind too :)

Shouldn't the call to failwith be replaced with

win32_maperr(GetLastError());
uerror("times", Nothing);

Cheers

(0006379)
jjb (reporter)
2011-12-18 15:48

Yes indeed, you are right about failwith.

Cheers, Josh
(0006388)
protz (manager)
2011-12-19 15:01

Hi,

First of all thanks for the patch. I just gave your code a try and it seems indeed to work right, both on a 32-bit windows and a 64-bit windows. I was a little worried about compiler support for the QuadWord, but mingw handles this fine.

We don't pretend to be compatible with anything earlier than Windows 2000 so this looks like a good idea to me. Can you submit a patch using the (work-in-progress) guidelines available at http://yquem.inria.fr/~protzenk/caml-faq/doku.php?id=How%20to%20contribute [^] ? Also, I'd like to see a comment explaining why you're not reporting on child processes (too hard on Windows?).

Also, the ULARGE_INTEGER is not needed in the body of the times function. Please remove its definition.

Thanks,

jonathan
(0006405)
jjb (reporter)
2011-12-20 11:44

Please see the attached patch against the current trunk.

The error reporting is done as suggested by ripoche, and the dead variable has been eliminated.

As for the statistics for child processes, the real reason is that I never needed that information so I hadn't implemented it. I could be missing something, but as far as I know Windows does not maintain this information itself. I don't know of a better way to compute this than to enumerate all process on the machine, (partially) reconstruct the process tree, query and sum the statistics for all children individually. At least in my context where Unix.times can be called frequently, I would want to be very careful about making so many, perhaps unnecessary, system calls.
(0006423)
protz (manager)
2011-12-20 22:09

Thanks! I'm about to commit this, please email me (jonathan.protzenko@inria.fr) your real name + email address for the commit message.
(0006428)
protz (manager)
2011-12-21 10:46

Thanks, the patch has been committed. Unfortunately, I was hoping that you would appear as the author, but svn doesn't allow you to commit as someone else (I was hoping that the git-svn bridge I'm using would allow that somehow). I'll make sure to include your name and email in the commit message next time.

This is svn r11912

Cheers,

jonathan

- Issue History
Date Modified Username Field Change
2011-12-18 14:22 jjb New Issue
2011-12-18 15:43 ripoche Note Added: 0006378
2011-12-18 15:44 ripoche Note Edited: 0006378 View Revisions
2011-12-18 15:48 jjb Note Added: 0006379
2011-12-19 15:01 protz Note Added: 0006388
2011-12-19 17:29 xleroy Status new => feedback
2011-12-20 11:44 jjb Note Added: 0006405
2011-12-20 11:44 jjb Status feedback => new
2011-12-20 11:45 jjb File Added: win32unix_times.patch
2011-12-20 22:09 protz Note Added: 0006423
2011-12-21 10:46 protz Note Added: 0006428
2011-12-21 10:46 protz Status new => resolved
2011-12-21 10:46 protz Resolution open => fixed
2012-09-25 20:07 xleroy Status resolved => closed


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker