Skip to content
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.

Sign up for GitHub

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

bpo-32025: Add time.thread_time() #4410

Merged
merged 6 commits into from
Nov 15, 2017
Merged

bpo-32025: Add time.thread_time() #4410

merged 6 commits into from
Nov 15, 2017

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Nov 15, 2017 •
edited by bedevere-bot
Loading

@pitrou pitrou changed the title [WIP] bpo-32025: Add time.thread_time() bpo-32025: Add time.thread_time() Nov 15, 2017
@pitrou pitrou requested a review from vstinner November 15, 2017 19:27
@pitrou pitrou added the type-feature A feature request or enhancement label Nov 15, 2017
vstinner
vstinner approved these changes Nov 15, 2017
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I just have two minor comments.

Doc/library/time.rst Outdated
returned value is undefined, so that only the difference between the results
of consecutive calls in the same thread is valid.

Availability: Unix systems supporting CLOCK_THREAD_CPUTIME_ID, Windows.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose:

Availability: Windows, Linux, Unix systems supporting CLOCK_THREAD_CPUTIME_ID.

Modules/timemodule.c Outdated
const clockid_t clk_id = CLOCK_THREAD_CPUTIME_ID;
const char *function = "clock_gettime(CLOCK_THREAD_CPUTIME_ID)";

if (clock_gettime(clk_id, &ts) == 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to invert the test to return early on error.

serhiy-storchaka
serhiy-storchaka reviewed Nov 15, 2017
View reviewed changes
Misc/NEWS.d/next/Library/2017-11-15-20-03-45.bpo-32025.lnIKYT.rst Outdated
@@ -0,0 +1 @@
Add time.thread_time()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And time.thread_time_ns().

Modules/timemodule.c
info->implementation = function;
info->monotonic = 1;
info->adjustable = 0;
if (clock_getres(clk_id, &res)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(clock_gettime(clk_id, &ts) != 0) but (clock_getres(clk_id, &res)).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really a copy-paste job.

Lib/test/test_time.py
start = time.thread_time()
busy_wait(0.100)
stop = time.thread_time()
self.assertGreaterEqual(stop - start, 0.020) # machine busy?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be 0.100 - 0.020 or maybe just 0.100?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't be too strict, or the test will fail randomly on grumpy buildbots.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole CPU time in those 0.1 seconds was not necessarily allocated to the current process / thread.

@pitrou pitrou merged commit 4bd41c9 into python:master Nov 15, 2017
@pitrou pitrou deleted the thread_time branch November 15, 2017 21:52
jimmylai added a commit to jimmylai/cpython that referenced this pull request Nov 17, 2017
@jimmylai
Merge branch 'master' of https://github.com/python/cpython
d914928 
* 'master' of https://github.com/python/cpython: (550 commits)
  bpo-31867: Remove duplicates in default mimetypes. (python#4388)
  tokenizer: Remove unused tabs options (python#4422)
  bpo-31691: Specify where to find build instructions for the Windows installer (python#4426)
  Fix typo in atexit documentation. (pythonGH-4419)
  bpo-31702: Allow to specify rounds for SHA-2 hashing in crypt.mksalt(). (python#4110)
  bpo-32043: New "developer mode": "-X dev" option (python#4413)
  bpo-30349: Raise FutureWarning for nested sets and set operations (python#1553)
  bpo-32037: Use the INT opcode for 32-bit integers in protocol 0 pickles. (python#4407)
  bpo-30143: 2to3 now generates a code that uses abstract collection classes (python#1262)
  bpo-32030: Enhance Py_Main() (python#4412)
  bpo-32030: Split Py_Main() into subfunctions (python#4399)
  bpo-32034: Make IncompleteReadError & LimitOverrunError pickleable python#4409
  bpo-32025: Add time.thread_time() (python#4410)
  bpo-32018: Fix inspect.signature repr to follow PEP 8 (python#4408)
  bpo-30399: Get rid of trailing comma in the repr of BaseException. (python#1650)
  bpo-30950: Convert round() to Argument Clinic. (python#2740)
  bpo-32011: Revert "Issue python#15480: Remove the deprecated and unused TYPE_INT64 code from marshal." (python#4381)
  bpo-32023: Disallow genexprs without parenthesis in class definitions. (python#4400)
  bpo-31949: Fixed several issues in printing tracebacks (PyTraceBack_Print()). (python#4289)
  bpo-32032: Test both implementations of module-level pickle API. (python#4401)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner approved these changes

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees
No one assigned
Labels
type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pitrou @vstinner @serhiy-storchaka @the-knights-who-say-ni @bedevere-bot