[issue1040026] os.times() is bogus

Martin v. Löwis added the comment: > Is this fixed in Python 2.6.1? Please try to answer this question yourself in the future. Python 2.6.1 was released on Dec 4th (http://www.python.org/download/releases/2.6.1/), and the commits were made on Dec 29 (as seen both in the time stamp on my message

[issue1040026] os.times() is bogus

Giampaolo Rodola' added the comment: Is this fixed in Python 2.6.1? We have encountered some problems on both OS X and FreeBSD by using 2.6.1: http://code.google.com/p/psutil/issues/detail?id=40 -- nosy: +giampaolo.rodola ___ Python tracker

[issue1040026] os.times() is bogus

Martin v. Löwis added the comment: Malte, thanks again for the patch; committed into the various branches as r68018, r68019, r68020, r68021. As annunciated, I reject the test; I don't think there is a reasonable way to test for this bug. -- status: open -> closed _

[issue1040026] os.times() is bogus

Changes by Martin v. Löwis : ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/py

[issue1040026] os.times() is bogus

Changes by Martin v. Löwis : -- versions: -Python 2.4, Python 2.5, Python 2.5.3 ___ Python tracker ___ ___ Python-bugs-list mailing

[issue1040026] os.times() is bogus

Changes by Martin v. Löwis : -- priority: deferred blocker -> release blocker ___ Python tracker ___ ___ Python-bugs-list mailing li

[issue1040026] os.times() is bogus

Martin v. Löwis added the comment: > I wouldn't be > surprised if it breaks on some machines sometimes, especially ones with > low clock granularity. Now that I look at the test, I see what you mean. I think I will reject it. ___ Python tracker

[issue1040026] os.times() is bogus

Changes by Guido van Rossum : -- nosy: -gvanrossum ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://ma

[issue1040026] os.times() is bogus

Guido van Rossum added the comment: [-gvanrossum] ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.

[issue1040026] os.times() is bogus

Malte Helmert added the comment: > I see. I wish that people would a) always provide complete patches in > a single file, and b) delete files themselves that have been > superceded by others. In any case, I have re-attached the file; > thanks for pointing this out. Regarding b), I did that as f

[issue1040026] os.times() is bogus

Martin v. Löwis added the comment: > should the unit test in test_posix5.patch have been removed? I see. I wish that people would a) always provide complete patches in a single file, and b) delete files themselves that have been superceded by others. In any case, I have re-attached the file;

[issue1040026] os.times() is bogus

Changes by Martin v. Löwis : Added file: http://bugs.python.org/file9542/test_posix5.patch ___ Python tracker ___ ___ Python-bugs-list maili

[issue1040026] os.times() is bogus

Gregory P. Smith added the comment: should the unit test in test_posix5.patch have been removed? (regardless, its still on the bug tracker via the history links) ___ Python tracker _

[issue1040026] os.times() is bogus

Changes by Martin v. Löwis : Removed file: http://bugs.python.org/file9542/test_posix5.patch ___ Python tracker ___ ___ Python-bugs-list mai

[issue1040026] os.times() is bogus

Changes by Martin v. Löwis : Removed file: http://bugs.python.org/file9541/posixmodule.diff ___ Python tracker ___ ___ Python-bugs-list mail

[issue1040026] os.times() is bogus

Changes by Martin v. Löwis : Removed file: http://bugs.python.org/file9518/os_times3.PATCH ___ Python tracker ___ ___ Python-bugs-list maili

[issue1040026] os.times() is bogus

Changes by Martin v. Löwis : Removed file: http://bugs.python.org/file9514/os_times2.PATCH ___ Python tracker ___ ___ Python-bugs-list maili

[issue1040026] os.times() is bogus

Martin v. Löwis added the comment: Committed os_times5.patch into 2.5 branch as r67739 Applying this to the other branches still needs to be done. -- assignee: gregory.p.smith -> loewis priority: release blocker -> deferred blocker resolution: -> accepted

[issue1040026] os.times() is bogus

Changes by Martin v. Löwis <[EMAIL PROTECTED]>: -- keywords: -easy ___ Python tracker <[EMAIL PROTECTED]> ___ ___ Python-bugs-list m

[issue1040026] os.times() is bogus

Changes by Martin v. Löwis <[EMAIL PROTECTED]>: -- priority: normal -> release blocker ___ Python tracker <[EMAIL PROTECTED]> ___ ___

[issue1040026] os.times() is bogus

Changes by Martin v. Löwis <[EMAIL PROTECTED]>: -- versions: +Python 2.5.3 ___ Python tracker <[EMAIL PROTECTED]> ___ ___ Python-bugs

[issue1040026] os.times() is bogus

Martin v. Löwis <[EMAIL PROTECTED]> added the comment: > compilation indeed breaks if sysconf is available but _SC_CLK_TCK is > not. My Unix-foo is not sufficient to confidently say that this is > impossible To make such a statement, one would need knowledge of all operating system releases that

[issue1040026] os.times() is bogus

Malte Helmert <[EMAIL PROTECTED]> added the comment: OK, modified and simplified patch attached (os_times5.PATCH). The patch and unit test (in test_posix5.PATCH) apply cleanly against the trunk. "make test" passes on two machines I tried, including a 64-bit Linux machine where the new unit test

[issue1040026] os.times() is bogus

Malte Helmert <[EMAIL PROTECTED]> added the comment: Martin, compilation indeed breaks if sysconf is available but _SC_CLK_TCK is not. My Unix-foo is not sufficient to confidently say that this is impossible, so my suggestion is to add defined(_SC_CLK_TCK) to the condition of this #ifdef branch.

[issue1040026] os.times() is bogus

Martin v. Löwis <[EMAIL PROTECTED]> added the comment: Malte proposed this patch for inclusion into Python 2.5. In its current form, I'm skeptical about doing so, as it has the potential for breakage. IIUC, the patch requires that HZ is defined if HAVE_TIMES is defined and HAVE_SYSCONF is not; p

[issue1040026] os.times() is bogus

Changes by Malte Helmert <[EMAIL PROTECTED]>: Removed file: http://bugs.python.org/file9540/os_times4.PATCH ___ Python tracker <[EMAIL PROTECTED]> ___ __

[issue1040026] os.times() is bogus

Changes by Malte Helmert <[EMAIL PROTECTED]>: Removed file: http://bugs.python.org/file9517/test_posix4.PATCH ___ Python tracker <[EMAIL PROTECTED]> ___

[issue1040026] os.times() is bogus

Changes by Malte Helmert <[EMAIL PROTECTED]>: Removed file: http://bugs.python.org/file9516/test_posix3.PATCH ___ Python tracker <[EMAIL PROTECTED]> ___

[issue1040026] os.times() is bogus

Changes by Malte Helmert <[EMAIL PROTECTED]>: Removed file: http://bugs.python.org/file9515/test_posix2.PATCH ___ Python tracker <[EMAIL PROTECTED]> ___

[issue1040026] os.times() is bogus

Changes by Malte Helmert <[EMAIL PROTECTED]>: Removed file: http://bugs.python.org/file9506/test_posix.PATCH ___ Python tracker <[EMAIL PROTECTED]> ___ _

[issue1040026] os.times() is bogus

Changes by Malte Helmert <[EMAIL PROTECTED]>: Removed file: http://bugs.python.org/file9501/os_times.PATCH ___ Python tracker <[EMAIL PROTECTED]> ___ ___

[issue1040026] os.times() is bogus

Changes by Malte Helmert <[EMAIL PROTECTED]>: Removed file: http://bugs.python.org/file9497/test_times.py ___ Python tracker <[EMAIL PROTECTED]> ___

[issue1040026] os.times() is bogus

Malte Helmert <[EMAIL PROTECTED]> added the comment: David, not sure what you are commenting on. Are you commenting on one of the patches? The patches do contain those divisions, of course; you can also run the attached unit test to verify that the patches work for you.

[issue1040026] os.times() is bogus

David W. Lambert <[EMAIL PROTECTED]> added the comment: I don't know what is "HZ", but if it's "hertz" then a division is necessary. total_clocks time = - clocks_per_second otherwise there's no hope. -- nosy: +LambertDW

[issue1040026] os.times() is bogus

Guido van Rossum <[EMAIL PROTECTED]> added the comment: Yes, please move this to 3.0.1 / 2.6.1. The hard part appears to be making sure that it compiles *and* is correct on all conceivable platforms... ___ Python tracker <[EMAIL PROTECTED]>

[issue1040026] os.times() is bogus

Gregory P. Smith <[EMAIL PROTECTED]> added the comment: I agree with Christian's most recent comment. However without BDFL intervention I think its too late in the 2.6/3.0 release cycle to rush this fix in. It can wait for 2.6.1/3.0.1. I won't have time to look at it for several days myself,

[issue1040026] os.times() is bogus

Malte Helmert added the comment: I think it's better only to only add another fallback if the unit tests show that such platforms exist. Avoiding cruft is important, too. After all, sysconf is a standard POSIX API, and from my (admittedly limited) research was already available in that form back

[issue1040026] os.times() is bogus

Hirokazu Yamamoto added the comment: How abount also check CLK_TCK? I know it's obsolute but maybe there are platforms without sysconf() and with CLK_TCK. -- nosy: +ocean-city _ Tracker <[EMAIL PROTECTED]> ___

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Great, we're approaching closure! :-) One final small thing: As said somewhere above, the allowed discrepancy in test_posix4.PATCH is a bit too low for machines with only 60 ticks per second. I uploaded a slightly more generous test_posix5.PATCH instead. So posi

[issue1040026] os.times() is bogus

Alexander Belopolsky added the comment: Thanks Cristian for your intervention. This bug is clearly an aberration: how many GvR reported bugs do you know that stayed open for 3+ years? I think posixmodule.diff satisfies all your requirements and was not opposed by anyone except yours truly.

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Christian, I agree on all points. Alexander's patch posixmodule.diff satisfies those requirements. I suggest also adding the unit test from os_times4.PATCH (but not the changes to posixmodule.c in that patch), as this will help identify misbehaving platforms in t

[issue1040026] os.times() is bogus

Christian Heimes added the comment: Guys, please don't waste too much time on this issue! The tracker still has more than 1,700 open issues for to debate on. While I enjoy the fact, that you three are enthusiastic, I strongly feel the urge to re-route your men power. This bug isn't important enou

[issue1040026] os.times() is bogus

Malte Helmert added the comment: In the first line of my previous message, I mean "#define HZ 60", of course. _ Tracker <[EMAIL PROTECTED]> _

[issue1040026] os.times() is bogus

Malte Helmert added the comment: If I remove the "#define 60" line in an unmodified posixmodule.c from trunk, I get the following compiler error: gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I. -IInclude -I./Include -DPy_BUILD_CORE -c ./Modules/posixmo

[issue1040026] os.times() is bogus

Alexander Belopolsky added the comment: > HZ may be defined on your machine, but it isn't on my (Xeon) machine. Are you sure? Please note that HZ will not show up in grep -w HZ /usr/include because the right header file further up the tree. On Solaris: /usr/include/sys/param.h:#define

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Alexander, your one-line patch *does* affect performance on my 64-bit Linux machine in a worse way than any other proposed patch by calling sysconf five times. HZ may be defined on your machine, but it isn't on my (Xeon) machine. I checked man pages on four diffe

[issue1040026] os.times() is bogus

Antoine Pitrou added the comment: > but I will oppose any changes that affect x86_64 linux performance. Alexander, as I said I'm really curious about any situation where os.times() is so performance-critical that a 5% slowdown for that function call is unacceptable. Even when a profiler is invo

[issue1040026] os.times() is bogus

Alexander Belopolsky added the comment: > You complained in msg62869 about the original patch that calling sysconf > on every call leads to an unacceptable slowdown. Your one-line patch > calls sysconf five times on each call when HZ is not defined. That's why I said that was a matter of person

[issue1040026] os.times() is bogus

Antoine Pitrou added the comment: I think it's better to make it a runtime error (upon invocation of os.times()), rather than a compile-time error. But it's quite theoretical until we find a system where the error does occur, anyway :) _ Tracker <[EMAIL PROTEC

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Antoine, none of the recently proposed patches uses the 60 magic value. Alexander's patch doesn't define times in that case (leading to an AttributeError when trying to call os.times) while my patch complains about the bogus environment during compilation. _

[issue1040026] os.times() is bogus

Malte Helmert added the comment: s/standard library/system library/, of course. Also, the original code is wrong in preferring HZ over the sysconf value. _ Tracker <[EMAIL PROTECTED]> _

[issue1040026] os.times() is bogus

Malte Helmert added the comment: >> I'd prefer a noisy compile error .. > > That would be fine if you could verify that none of the currently > supported platforms will be affected. I would still feel uneasy about > refusing to build python simply because os.times is not ported to a > platform

[issue1040026] os.times() is bogus

Antoine Pitrou added the comment: IMO, if there is no available way to compute HZ, a NotImplementedError should be raised rather than using the 60 magic value. _ Tracker <[EMAIL PROTECTED]>

[issue1040026] os.times() is bogus

Alexander Belopolsky added the comment: > I'd prefer a noisy compile error .. That would be fine if you could verify that none of the currently supported platforms will be affected. I would still feel uneasy about refusing to build python simply because os.times is not ported to a platform.

[issue1040026] os.times() is bogus

Malte Helmert added the comment: I'd prefer a noisy compile error, since in situations where times is available but unusable, HAVE_TIMES shouldn't have been #defined in the first place. (That is, I'd see that as a bug in the configure script.) But this is turning into a bikeshed discussion. I ca

[issue1040026] os.times() is bogus

Alexander Belopolsky added the comment: Here is my take (posixmodule.diff). When ticks per seconds cannot be determined, python should not provide times rather than refuse to build. Initialization moved to the init function. -- keywords: +patch Added file: http://bugs.python.org/file

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Here is an updated patch (os_times4.PATCH) that incorporates Christian's suggestions. The patch includes the new unit test, so test_posix?.PATCH need not be applied separately. I again made the unit test a bit more lenient to allow an absolute error of 0.02 secon

[issue1040026] os.times() is bogus

Christian Heimes added the comment: I don't *use* Windows except for some computer games. But I'm doing some development for Python on Windows in a VMWare box. I'm going to check HZ for you. Give me a couple of minutes to boot the image. How do you like this idea. Since HZ is only used in posix_

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Never mind, on Windows a different code path is chosen. I'm now working on a patch that computes the hz value during module initialization. So should I keep the 60 magic value? What is the justification? _ Tracker <[EMAIL PROTE

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Tiran, that's the general approach we should follow, yes. But the people who discussed this on #python-dev all felt a bit queasy about the "60" fallback -- this is what caused the bug in the first place on Guido's and my machine. (A value of 60 was assumed; 100 w

[issue1040026] os.times() is bogus

Christian Heimes added the comment: I suggest that you define "static int hz" and assign a value to the var in INITFUNC(). #ifdef HZ hz = HZ; #elif defined(HAVE_SYSCONF) hz = sysconf(_SC_CLK_TCK); #else hz = 60; /* It's 50Hz in Europe */ #endif -- nosy: +tiran _

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Alexander, speed-wise your patch is worse than the original one on systems where HZ isn't predefined, because it calls sysconf 5 times in each call to os.times, rather than only once per call. If you worry about speed, the approach outlined in Antoine's last mess

[issue1040026] os.times() is bogus

Antoine Pitrou added the comment: Hi Alexander, > 5% is a lot and IIRC os.times is used by some standard python profilers > and 5% slowdown will affect people. Profiled runs are always slower than non-profiled runs, so I'm not convinced it is very important. You use profiling only when you nee

[issue1040026] os.times() is bogus

Alexander Belopolsky added the comment: FWIW, the following minimal patch fixed the bug on MacOS X and does not affect Linux: === --- Modules/posixmodule.c (revision 61014) +++ Modules/posixmodule.c (working copy) @@ -5

[issue1040026] os.times() is bogus

Alexander Belopolsky added the comment: Aha, I should read my own sources: "The value shall not change during the lifetime of the calling process, [XSI] except that sysconf(_SC_OPEN_MAX) may return different values before and after a call to setrlimit() which changes the RLIMIT_NOFILE soft

[issue1040026] os.times() is bogus

Alexander Belopolsky added the comment: > 1. .. It's preferable to cast into a clock_t immediately rather than > doing a conversion for each of the ensuing divisions. If that's your motivation, then you should cast to double instead. However, I would leave it to compiler to do micro-optimizatio

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Alexander, regarding your comments: 1. sysconf in general returns a long because it can return all sorts of information, but os.times() returns clock_t items, so the _SC_CLK_TCK value must comfortably fit into a clock_t. It's preferable to cast into a clock_t imm

[issue1040026] os.times() is bogus

Changes by Alexander Belopolsky: _ Tracker <[EMAIL PROTECTED]> _ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailm

[issue1040026] os.times() is bogus

Alexander Belopolsky added the comment: os_times3.PATCH works for me on Mac OS 10.4 and RHEL. I have a few comments on the patch: 1. sysconf return type is long, not clock_t 2. If sysconf is present, but _SC_CLK_TCK is not supported, it will return -1. In this case we should fall back to sys

[issue1040026] os.times() is bogus

rbp added the comment: Malte noticed that my previous patch won't compile when HAVE_SYSCONFIG and HZ are not defined. My bad, silly mistake. I've attached a new version, which compiles and has been tested on all three cases (with test_posix4.PATCH). Please, someone with privileges remove os_time

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Sorry, but the test was still wrong because I misunderstood how assertAlmostEqual works. Attaching a fourth (final?) test. Added file: http://bugs.python.org/file9517/test_posix4.PATCH _ Tracker <[EMAIL PROTECTED]>

[issue1040026] os.times() is bogus

Malte Helmert added the comment: I was wrong -- 0.1 isn't enough, because os.times() typically has 0.01s resolution, so we can easily get 0.1 vs. 0.11 which will fail the assertion. Cranked up the WAIT_TIME to 0.3 in the attached patch (test_posix3.PATCH). Sorry for the noise. Added file: http:/

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Attaching a new test (test_posix2.PATCH) that doesn't fork and fixes the problem with the previous test not taking previously elapsed time into account. This supersedes test_posix.PATCH. I left the wait time at 0.1; if we stay within the same process, this should

[issue1040026] os.times() is bogus

rbp added the comment: Malte, Antoine and I discussed this a bit on #python-dev and concluded that the correct behaviour should be trying sysconf first, then HZ, or raise an exception if not even HZ is available (since whichever static value we chose would be misleading anyway). I'm attaching a

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Note: My original unit test fails to take account that previous unit tests may have spawned child processes. The correct behaviour is of course to call os.times() before and after the fork and compute the difference. I'm not uploading a modified patch because fro

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Using 1.0 would certainly be more robust. I wasn't sure if a slow-down of "make test" by 1 second just for this one bug would be acceptable. Regarding the fork, when I first encountered this bug, it was in the context of measuring the runtime of child processes,

[issue1040026] os.times() is bogus

Antoine Pitrou added the comment: Well first I can't reproduce the bug on my machine :) However the two patches do not produce any regression either. I have some questions: 1. isn't 0.1 for WAIT_TIME a bit too low? 1.0 would probably be less fragile IMHO 2. why do you fork() in test_times, rather

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Here is a patch that adds a test case to test_posix.py. This is ignored on Windows, as it requires fork. There is a trade-off: If WAIT_TIME isn't large enough, small irregularities in the process scheduler might cause this to fail. If it is too large, the unit te

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Another comment: Since the fallback value of 60 was wrong in the past, it may likely be wrong in the future. Should that fallback be removed and replaced by a compile-time error? And is the "HZ" fallback necessary at all? I don't know enough about Posix to know wh

[issue1040026] os.times() is bogus

Malte Helmert added the comment: I'm attaching a patch against trunk that fixes the problem for me (os_times.PATCH). This uses the sysconf values when HAVE_SYSCONF is defined, and otherwise falls back on the old behaviour (use HZ if that is defined, 60 otherwise). I'm not sure if this is styli

[issue1040026] os.times() is bogus

Malte Helmert added the comment: Here's three tests with different pythons on the same machine: # ./python ../test_times.py 8.333 # python ../test_times.py 8.333 # python2.5 ../test_times.py 5.0 The first Python is current trunk, built just now. The second Python is the vendor-in

[issue1040026] os.times() is bogus

Guido van Rossum added the comment: test_times.py produces the correct value on Linux for me, but I see the same bogus value as Malte on OSX. _ Tracker <[EMAIL PROTECTED]> _

[issue1040026] os.times() is bogus

Guido van Rossum added the comment: Well, 8./5 equals 100/60. Go figure. _ Tracker <[EMAIL PROTECTED]> _ ___ Python-bugs-list mailing list

[issue1040026] os.times() is bogus

Malte Helmert added the comment: I'm attaching a test script (test_times.py) that forks a child which runs for 5 seconds, waits for the child, then prints the time taken by the child according to os.times(). I have a machine where os.times() reproducably reports that 8.33 seconds have been spent

[issue1040026] os.times() is bogus

Alexander Belopolsky added the comment: On i386 Linux, HZ is #defined as sysconf(_SC_CLK_TCK) /usr/include/asm-i386/param.h:7:#define HZ sysconf(_SC_CLK_TCK) so times does the right thing. On x86_64 HZ is defined as 100, but it is the same value as sysconf returns. I could not find an author

[issue1040026] os.times() is bogus

Changes by A.M. Kuchling: -- keywords: +easy _ Tracker <[EMAIL PROTECTED]> _ ___ Python-bugs-list mailing list Unsubscribe: http://