Unix.times unimplemented/suboptimal on Windows #5434
Comments
|
Comment author: ripoche I had something like that in mind too :) Shouldn't the call to failwith be replaced with win32_maperr(GetLastError()); Cheers |
|
Comment author: @jberdine Yes indeed, you are right about failwith. Cheers, Josh |
|
Comment author: @protz 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 |
|
Comment author: @jberdine 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. |
|
Comment author: @protz Thanks! I'm about to commit this, please email me (jonathan.protzenko@inria.fr) your real name + email address for the commit message. |
|
Comment author: @protz 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 |
Original bug ID: 5434
Reporter: @jberdine
Status: closed (set by @xavierleroy on 2012-09-25T18:07:21Z)
Resolution: fixed
Priority: normal
Severity: feature
Version: 3.12.1
Category: ~DO NOT USE (was: OCaml general)
Monitored by: @protz @alainfrisch
Bug description
Here 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;
}
File attachments
The text was updated successfully, but these errors were encountered: