You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Rush Manbert (JIRA)" <ji...@apache.org> on 2009/05/08 00:12:45 UTC
[jira] Created: (THRIFT-491) Ripping raw pthreads out of
TFileTransport and associated test issues
Ripping raw pthreads out of TFileTransport and associated test issues
---------------------------------------------------------------------
Key: THRIFT-491
URL: https://issues.apache.org/jira/browse/THRIFT-491
Project: Thrift
Issue Type: Question
Components: Library (C++)
Affects Versions: 0.1
Environment: Mac OS X 10.5.6
Reporter: Rush Manbert
The last piece of replacing the pthread-based threading implementation with a Boost threads implementation is to replace all of the raw ptherad mutex and condition usage in TFileTransport.
I think the best way to proceed is to define a Condition class, similar to the way there is a Mutex class defined, give it a generic API that satisfies the demands of TFileTransport, then implement it both ways. You would need to construct a Condition object with a Mutex object reference, or probably a weak_ptr<Mutex> so we can know that the condition waits are safe. I'll add a comment with the API I work out.
However, my main concern is how to test the new code. It looks like I can use stress-test, but iI was hoping that someone who is familiar with the app could give me some pointers or a test procedure that exercises the threading code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-491) Ripping raw pthreads out of
TFileTransport and associated test issues
Posted by "Rush Manbert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-491?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707187#action_12707187 ]
Rush Manbert commented on THRIFT-491:
-------------------------------------
Rather than defining a new Condition class, it turns out to make a lot more sense to just add this public method to Monitor:
/** Create a new Monitor that has a different condition, but shares the mutex with this one.
*/
shared_ptr<Monitor> newMonitorSharedMutex() const;
and change the implementation to keep a boost::shared_ptr to the mutex object. This allows three Monitor objects to be used in TFileTransport in place of the three pthread_cond_t objects that it has now.
It looks like I need to define a new class that just does basic threading, either pthreads or boost threads. That will take care of the writer thread.
Comments?
> Ripping raw pthreads out of TFileTransport and associated test issues
> ---------------------------------------------------------------------
>
> Key: THRIFT-491
> URL: https://issues.apache.org/jira/browse/THRIFT-491
> Project: Thrift
> Issue Type: Question
> Components: Library (C++)
> Affects Versions: 0.1
> Environment: Mac OS X 10.5.6
> Reporter: Rush Manbert
>
> The last piece of replacing the pthread-based threading implementation with a Boost threads implementation is to replace all of the raw ptherad mutex and condition usage in TFileTransport.
> I think the best way to proceed is to define a Condition class, similar to the way there is a Mutex class defined, give it a generic API that satisfies the demands of TFileTransport, then implement it both ways. You would need to construct a Condition object with a Mutex object reference, or probably a weak_ptr<Mutex> so we can know that the condition waits are safe. I'll add a comment with the API I work out.
> However, my main concern is how to test the new code. It looks like I can use stress-test, but iI was hoping that someone who is familiar with the app could give me some pointers or a test procedure that exercises the threading code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-491) Ripping raw pthreads out of
TFileTransport and associated test issues
Posted by "Rush Manbert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-491?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12708245#action_12708245 ]
Rush Manbert commented on THRIFT-491:
-------------------------------------
The changes to Monitor look good and work as expected and there's now a Monitor test.
I got looking at the writerThread in TFileTransport. Initially, I thought I'd define a new Runnable-derived class and move the thread method into it as its run() method. The problem with that is that the thread function uses lots of the data members and functions in TFileTransport.
I am now considering whether I can just add Runnable as a base class of TFileTransport, rename writerThread() as run(), and just go with the "mimimum disturbance" approach. The biggest problem that I see is that Iwould neede to add boost::enable_shared_from_this as a base class and use it to create the Thread, without being able to guarantee that the TFileTransport object is owned by a boost::Shared_ptr.
I could solve that by taking the constructor private and providing a static method that returns bost::shared_ptr<TFileTransport>, but that breaks existing code.
The nastier route is to do something like my original plan. Make a separate Runnable class for the writerThread, and construct it with a pointer to the TFileTransport that owns it, then make the thread class be a friend of TFileTransport and just reach in as if run() were a member of TFileTransport. If I wanted to make it cleaner I could put all the shared data in a separate structure that is shared between the writerThread class and the TFileTransport, and make sure that TFileTransport has an API that supports all the calls that writerThread needs to make. It's more work, and more disturbance to the original design, but maybe safer. Or maybe not.
Does anyone with knowledge of TFileTransport care to comment? Please?
> Ripping raw pthreads out of TFileTransport and associated test issues
> ---------------------------------------------------------------------
>
> Key: THRIFT-491
> URL: https://issues.apache.org/jira/browse/THRIFT-491
> Project: Thrift
> Issue Type: Question
> Components: Library (C++)
> Affects Versions: 0.1
> Environment: Mac OS X 10.5.6
> Reporter: Rush Manbert
>
> The last piece of replacing the pthread-based threading implementation with a Boost threads implementation is to replace all of the raw ptherad mutex and condition usage in TFileTransport.
> I think the best way to proceed is to define a Condition class, similar to the way there is a Mutex class defined, give it a generic API that satisfies the demands of TFileTransport, then implement it both ways. You would need to construct a Condition object with a Mutex object reference, or probably a weak_ptr<Mutex> so we can know that the condition waits are safe. I'll add a comment with the API I work out.
> However, my main concern is how to test the new code. It looks like I can use stress-test, but iI was hoping that someone who is familiar with the app could give me some pointers or a test procedure that exercises the threading code.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.