[proxy] web.archive.org — ← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] bpo-36205: incorrect time.process_time when built on macOS < 10.12 #12287

Closed
wants to merge 1 commit into from
Closed

[WIP] bpo-36205: incorrect time.process_time when built on macOS < 10.12 #12287

wants to merge 1 commit into from

Conversation

Copy link
Member

ned-deily commented Mar 12, 2019 •
edited

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 requested a review from vstinner Mar 12, 2019
bedevere-bot added tests Tests in the Lib/test dir awaiting merge labels Mar 12, 2019
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
Copy link
Member

vstinner commented Mar 12, 2019

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?

Copy link
Member Author

ned-deily commented Mar 12, 2019

@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.

Copy link
Member Author

ned-deily commented Mar 12, 2019

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.

Copy link
Member

vstinner commented Mar 14, 2019

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...

Copy link
Member Author

ned-deily commented Aug 27, 2019

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 closed this Aug 27, 2019
ned-deily deleted the bpo-36206_macos_process_time branch May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

vstinner

Assignees
No one assigned
Labels
awaiting merge DO-NOT-MERGE needs backport to 3.7 skip news tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants