CERTI - Patches: patch #7704, replace local full-weight...
You are not allowed to post comments on this tracker with your current authentication level.
patch #7704: replace local full-weight processes with threads
Submitter: | Eugene Chemeritskiy <tyz> | ||
Submitted: | Fri 20 Jan 2012 05:56:00 AM UTC | ||
Category: | CERTI | Priority: | 5 - Normal |
Status: | Postponed | Privacy: | Public |
Assigned to: | None | Open/Closed: | Open |
( Jump to the original submission )
Sat 25 Aug 2012 01:15:26 PM UTC, comment #9: |
Eugene Chemeritskiy <tyz>![]() |
Sat 25 Aug 2012 12:57:28 PM UTC, comment #8: Hi Eugene,
I just created the branch.
Please be gentle with the commit access, Savannah does not
So you have commit right "globally", I just ask you
Are you OK with that?
|
Eric NOULARD <erk>![]() |
Sat 25 Aug 2012 11:43:48 AM UTC, comment #7: Eric,
It seems that I somehow misunderstood you. Of course I do not have permission to make a branch in CERTI CVS rep. Could you, please, make br_DEV_CERTI_threaded branch for me? |
Eugene Chemeritskiy <tyz>![]() |
Fri 24 Aug 2012 07:16:04 AM UTC, comment #6: Hi Eric,
I've created independent libRTIA subdirectory, because in addition to both RTIA and libRTI multi-threaded LRC includes some extra sources. Thus, it results into some kind of new CERTI component and deserves the similar separation. However, I do not have any objections of moving it anywhere. As about the future prospective, I think it will be a nice catch to include multi-threaded LRC not only as a build, but as a run time option either. The internal RTIA-RTIG protocol is remains the same, so one execution can include both multi-process and multi-threaded LRCs. Thus, it seems quite reasonable to separate RTIA core into an independent library for convenient use in both cases.
Here I tried to express that I knew how to implement the considerably clean and correct solution in case of related waiting as well as I knew nothing about internal CERTI component communications. So I've made a kind of kludge to get the proof-of-concept multi-threaded LRC.
I understand your reluctance of including dependency on third-party library in CERTI. However, it seems quite strange to reject boost usage. Currently multi-threaded LRC version is considered as a build option, so the legacy configuration will not require boost at all. Although it is not too difficult to implement threading for one platform only, it will discriminate users of other platforms. If the user really wants to get multi-threaded LRC, he can easily get boost library for an appropriate platform. But there is no hope to do it in case of platform dependent code. Another option is to implement your own compile-time condition-based code for multiple platforms. Currently there are several lines of code in this patch, but the idea does not seem to be a fast fix already. Moreover, if you will try to use threads inside of RTIG in future, you likely require a mature full-featured threading library, and its development will be a crucial overhead for a small researcher group. To be honest, I do not think that it is a good idea to reject boost thread (or any of its competitors) even for now.
The development plan you suggested is quite reasonable. Thanks for your review and constructive criticism. I'm going to make the new branch as soon as I get enough time for it. |
Eugene Chemeritskiy <tyz>![]() |
Tue 21 Aug 2012 12:59:29 PM UTC, comment #5: Hi Eugene,
...Patch review continued...
Concerning libRTIA or no libRTIA, if the code in there
If ever the code is independent enough, like
LibHLA may contains any RTI-agnostic code which could be useful
by the way the naming "MsgBuf" for your class is unfortunately
I suggest to name it "TSInMemoryMsgQueue" or something similar.
TSInMemoryMsgQueue being TSInMemoryQueue<certi::MessageBuffer*>.
You said:
I don't get it what do you mean?
I think you get all my remark now.
So I can propose you to create a br_DEV_CERTI_threaded
1) Try to avoid boost usage
2) Merge part of it to HEAD when we are ok.
3) In the end go back to HEAD with a unified approach.
Would you be OK with this process?
|
Eric NOULARD <erk>![]() |
Mon 20 Aug 2012 06:41:24 AM UTC, comment #4: Hi Eric,
Thanks for your comments.
Built-in CERTI tests require some extra packages and I had troubles while building it on PC I did not have root. I agree this part do not correspond the threading topic at all. It is my fault this code leaked into proposal.
I agree the positive logic is easier to understand. I think the reason I wrote it negative is a custom to write include guard in C-headers.
I absolutely agree about the cmake include options. The center question for me in here is whether to add libRTI directory. If you think that it is an appropriate build solution, I am ready to modify the patch in a corresponding way.
The polling I used is also an attempt to set aside the problem I know how to solve in favor of the those I do not. In general I had to deal with the internal message protocol between RTIA and libRTI. The standard approach is to remove "select" and add the third thread to get usually more efficient related waiting as a result. If you have the similar feeling I can have a look on it either.
I am also consider the ring buffer to be a good approach, however currently it seems to be the second place task. In my opinion, we should made a working patch first. Thus the straightforward, clear and reliable boost mutex-guard implementation is the current best.
Looking for the following comments,
|
Eugene Chemeritskiy <tyz>![]() |
Thu 16 Aug 2012 10:14:14 AM UTC, comment #3: Hi Eugene,
I did have a more thorough look to your patch.
A small nick-pick.
#ifndef RTIA_USE_THREAD
should be easier to understand like:
This are the same but it's usually easier to catch
This
or use CMake "include_directories" command.
The usage of Boost lock, guard etc... for libRTIABuf
The main pb with this is the polling mechanism you have to
I shall stop the review here.
|
Eric NOULARD <erk>![]() |
Fri 20 Jan 2012 09:32:49 AM UTC, comment #2: Hi Eric,
Thank you for your fast and detailed reply.
Answering your first question, I have tested proposed patch on two tests. Each test federations consists of message [interaction] sender and registrar. Sender creates series of messages containing single numerical parameter. The initial positive value is set to parameter during the test initialization, and is decremented with each message send until it reaches zero value. During the first test the registrar just observes the passed parameter values. In case of the second test the registrar also replies to each message with its full copy,
Considering the benchmarking, for this purpose I have some python
Considering your second question, primary target architecture is linux, but I am looking for a cross-platform solution in the first place. |
Eugene Chemeritskiy <tyz>![]() |
Fri 20 Jan 2012 07:22:28 AM UTC, comment #1: Hi Eugene,
This looks like a nice piece of patch, thanks.
1) Making the multi-threaded compilation
2) We try hard not to make CERTI source code dependent
3) Concerning the two-sided activity of RTIA
Those threads may share some in-memory ring buffer
4) A student (Jean-Baptiste)
5) I think the message based approach between
Some questions:
Q2) Which host platform do you target primiraly?
That my preliminary remarks.
More comment on the patch itself in a couple of day.
|
Eric NOULARD <erk>![]() |
Fri 20 Jan 2012 05:56:00 AM UTC, original submission:
This patch slightly changes CERTI RTI process model in attempt to increase overall RTI performance. Currently local RTI component consists of two full-weight processes, namely RTIA and Federate process. These processes interact to each other by means of TCP or UNIX socket (UNIX pipe). This type of interaction requires a lot of repeated message representation transitions and seems not to be an optimal one. There are a lot of suitable approaches: for example using of shared memory can partially solve the issue. The same common address space benefit could be reached by means of multi-threaded program.
Patch includes minor architectural changes only. It leaves unchanged the existing logical separation of duties between processes and uses the same message-based approach for local on host synchronization. Subsequently these mechanisms should be replaced due to its ineffectiveness, but all in good time.
The next issue that deserves some consideration is a simultaneous polling of Federate and RTIG in RTIA thread.
Technically patch adds an option 'RTIA_USE_THREAD' that allows building of multi-threaded architecture whereas the traditional multi-process architecture is still available by default. Multi-threading is reached using cross-platform boost::thread library.
There are a lot of issues to be solved, and I will be glad to discuss the proposal, the possibility of its integration into the main CERTI development branch, and the prospective of the further development in the specified direction. |
Eugene Chemeritskiy <tyz>![]() |
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Eric,
Thanks for your fast reaction.
As about global write access, I will keep this in mind and double check my current branch before commits.