Skip to content

Conversation

@ned-deily
Copy link
Member

@ned-deily ned-deily commented Mar 12, 2019

bpo-36205 documents a problem where time.process_time and time.perf_counter return very different values when built on macOS 10.11 or earlier. Several time related functions were added to macOS at 10.12 including clock_gettime. For older systems, timemodule.c falls back to using getrusage. With Python 3.6.x, that fallbacks correctly but it appears that refactoring introduced with the implementation of PEP 564 (bpo-31784, #3989) broke that for 3.7.z.

This initial PR just includes a potential test case to ensure process_time and perf_counter return similar results. It can be used to reproduce the problem when building on newer versions of macOS (10.12+) by removing the AC_CHECK_FUNCS(clock_gettime) test in configure.ac:

diff --git a/configure.ac b/configure.ac
index 73ee71c6d2..0c61a088d9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3853,15 +3853,6 @@ char *r = crypt_r("", "", &d);
     [])
 )

-AC_CHECK_FUNCS(clock_gettime, [], [
-    AC_CHECK_LIB(rt, clock_gettime, [
-        LIBS="$LIBS -lrt"
-        AC_DEFINE(HAVE_CLOCK_GETTIME, 1)
-        AC_DEFINE(TIMEMODULE_LIB, [rt],
-                  [Library needed by timemodule.c: librt may be needed for clock_gettime()])
-    ])
-])
-
 AC_CHECK_FUNCS(clock_getres, [], [
     AC_CHECK_LIB(rt, clock_getres, [
         AC_DEFINE(HAVE_CLOCK_GETRES, 1)

and running the test in this PR:

autoconf && ./configure -q && make -s && \
./python.exe -m test test_time -m test_perf_counter_vs_process_time

https://bugs.python.org/issue36205

@ned-deily ned-deily requested a review from vstinner March 12, 2019 09:39
@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting merge labels Mar 12, 2019
@ned-deily ned-deily changed the title [WIP] bpo-36206: incorrect time.process_time when built on macOS < 10.12 [WIP] bpo-36205: incorrect time.process_time when built on macOS < 10.12 Mar 12, 2019
@vstinner
Copy link
Member

I am not excited by tests on clocks. In the past, I had to remove such tests which were too fragile.

Would it make sense to add a test on time.get_clock_info()? Maybe a test specific to macOS?

@ned-deily
Copy link
Member Author

@vstinner I am usually not excited by clock tests either. However, I was looking for you to investigate why process_time has been broken in 3.7 :) Also, I would think that this problem might be seen on other platform/versions that lack clock_gettime.

@ned-deily
Copy link
Member Author

Good call, @vstinner! It looks like the test case as it stands is too fragile: it failed on the Azure Pipeline win64.

FAIL: test_perf_counter_vs_process_time (test.test_time.TimeTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\1\s\lib\test\test_time.py", line 512, in test_perf_counter_vs_process_time
    self.assertAlmostEqual(processtime, perfcounter, places=2)
AssertionError: 0.140625 != 0.15217669999992722 within 2 places (0.011551699999927223 difference)

Well, it should still be useful in figuring out what the problem is. We can decide later whether to make it more robust and add to the repo.

@vstinner
Copy link
Member

Yeah, please don't add such test which rely too much on clock resolution/accuracy. These tests fail often in the past, it's very painful to fix them. For example, I modified some tests to tolerate a delta of 1 second on a duration which should be 20 ms... Such test doesn't make an sense anymore...

@ned-deily
Copy link
Member Author

I still think it would be better to have a test for this case since the problem embarrassingly went undetected for quite some time. But I'll let some one else deal with it if they care to.

@ned-deily ned-deily closed this Aug 27, 2019
@ned-deily ned-deily deleted the bpo-36206_macos_process_time branch May 19, 2020 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants