Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(17)

Issue 150055: [issue3001] RLock's are SLOW

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 9 months ago by gregory.p.smith
Modified:
12 years, 9 months ago
Reviewers:
Antoine Pitrou
CC:
report_bugs.python.org
Base URL:
http://svn.python.org/view/*checkout*/python/branches/py3k/
Visibility:
Public.

Description

code review for http://bugs.python.org/issue3001

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+273 lines, -7 lines) Patch
M Lib/test/test_threading.py View 3 chunks +8 lines, -5 lines 0 comments Download
M Lib/threading.py View 3 chunks +12 lines, -2 lines 0 comments Download
M Modules/_threadmodule.c View 3 chunks +253 lines, -0 lines 4 comments Download

Messages

Total messages: 2
gregory.p.smith
http://codereview.appspot.com/150055/diff/1/4 File Modules/_threadmodule.c (right): http://codereview.appspot.com/150055/diff/1/4#newcode221 Modules/_threadmodule.c:221: return PyBool_FromLong((long) r); This explicit (long) cast is unnecessary. ...
12 years, 9 months ago (2009-11-07 07:48:04 UTC) #1
Antoine Pitrou
12 years, 9 months ago (2009-11-07 15:06:51 UTC) #2
Thanks for the review. I will make the suggested modifications.

http://codereview.appspot.com/150055/diff/1/4
File Modules/_threadmodule.c (right):

http://codereview.appspot.com/150055/diff/1/4#newcode221
Modules/_threadmodule.c:221: return PyBool_FromLong((long) r);
On 2009/11/07 07:48:05, gregory.p.smith wrote:
> This explicit (long) cast is unnecessary.

Right.

http://codereview.appspot.com/150055/diff/1/4#newcode246
Modules/_threadmodule.c:246: PyThread_release_lock(self->rlock_lock);
On 2009/11/07 07:48:05, gregory.p.smith wrote:
> reset self->rlock_owner to 0 before releasing the lock.

We always check rlock_count before rlock_owner anyway but, yes, I could reset
rlock_owner out of safety.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b