Issue1846247
Created on 2007-12-07.15:08:42 by mehendran, last changed 2007-12-12.11:50:25 by mehendran.
msg2966 (view) |
Author: Mehendran (mehendran) |
Date: 2007-12-07.15:08:42 |
|
I have implemented thread-local support on jython.
I have included the py based thread-local
implementation also as in the link
http://wiki.python.org/jython/ThreadLocalVariables.
I have written a testcase file(test_thread_local.py)
based on the test_threading_local.py for testing the
java based thread-local implementation.
Test files:
1) test_thread_local.py
2) test_threading_local.py
The second test case will be failing because the string
representation of thread-local object is not as expected.
And DocTestSuite.addSuite is expecting only one argument. So I commented the two arguments in
test_threading_local.py in the calling place of
addSuite method.
Actually _threading_local.py module is attached with
the doctest and that declares MyLocal, subclass of
local type. There will be a test case expecting the
string "AttributeError: 'MyLocal' object has no
attribute 'color'". But jython gives
<module_name>.MyLocal in place of MyLocal.
<module_name> = __main__ or
= the file name where the class is
declared.
This doctest is used by test_threading_local.py
We may avoid this error by overriding the safeRepr in
PyLocal.java. For py based implementation, we have to step into PyObject.java. I dont think avoiding error is
correct. It might have been implemented for some
purpose. The another way is changing the test case.
Please give me your valuable suggestions/comments.
|
msg2967 (view) |
Author: Charlie Groves (cgroves) |
Date: 2007-12-09.00:58:26 |
|
Because ThreadLocal tdict is static in PyLocal, making a second threading.local object in a single thread destroys the dict from the first local object. Run the following code with your patch and then with CPython to see what I mean:
import threading
first = threading.local()
first.x = 7
print "After creation:", first.x
second = threading.local()
print "After creating a second:", first.x
I've attached a version of the patch that fixes that, and cleans up the logic in local_init a bit. The way you'd written it, an initless subclass of a subclass of local that had an init would be unable to take args or kwargs even though it has a superclass that can reasonably handle them.
I also added tests for that sort of sublclassing and the use of two locals in a single thread to your test_thread_local.py. I think we can wait to bring _threading_local.py over to our Lib whenever we fix that module naming problem. It seems like your test does everything in the doctest, and there's no need for _threading_local as a backup since we're always going to include PyLocal. It might be worth filing a bug for the doctest problem though.
The only thing that's keeping me from committing my version is that I'm not sure where the version of thread.py you supplied came from. What version of CPython was that in? It's not 2.5: that version uses deque. As you well know, Jython doesn't have an implementation of deque quite yet :) It probably will soon though...
One last thing: please add the .py files you want to add to Lib like I did in my patch unless they're identical to the ones in CPythonLib. In that case, add them to CPythonLib.includes as you did for _threading_local.py It's easier for me to figure out what merging in I need to do if they land in Lib.
File Added: thread_local_groves.diff
|
msg2968 (view) |
Author: Mehendran (mehendran) |
Date: 2007-12-10.08:37:02 |
|
Thanks. As you said, I ran your test with the old patch.
The second call is overwriting the existing local. You
have solved that. Good!
I used the same test cases from _threading_local.py as
I mentioned in the comments.
I saw that threading.py of CPython2.4.4 uses deque. But
I didn't copy the threading.py from any version of CPython.
It was already in CPython/Lib folder. I just inlcuded the
necessary code to run the thread local. I thinks it is
from http://svn.python.org/projects/python/branches/release23-maint/Lib.
I thought that there is not much change in the following files.
CPythonLib/threading.py
CPythonLib/_threading_local.py
CPythonLib/test/test_threading_local.py
That's why I copied to CPython/Lib. I will follow your comments
from now on.
I don't know why you changed the code. You could have suggested/
commented that and I would have done those. Maybe, you thought to
avoid unnecessary thread mailing. That's fine.
|
msg2969 (view) |
Author: Charlie Groves (cgroves) |
Date: 2007-12-12.09:41:59 |
|
Ahh, I see that threading.py is from CPythonLib now. Dunno how I got confused there. Like you said, I'm only changing it myself because I'd pretty much already done it trying to understand what was going on, so I didn't see any point in asking you to do the same thing.
In any case, I applied the patch in r3801. Thanks!
|
msg2970 (view) |
Author: Mehendran (mehendran) |
Date: 2007-12-12.11:50:25 |
|
Thanks. Nice to hear the good news.
|
|
Date |
User |
Action |
Args |
2007-12-07 15:08:42 | mehendran | create | |
|