Bugzilla – Full Text Bug Listing
| Summary: | [rosgi] System is out of thread resources when using r_osgi provider | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [RT] ECF | Reporter: | ronen hamias <ronen.hamias> | ||||
| Component: | ecf.providers | Assignee: | Scott Lewis <slewis> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | slewis, wim.jongman | ||||
| Version: | 3.5.0 | Keywords: | performance | ||||
| Target Milestone: | 3.5.1 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
i have started to work on a patch. i need to pass the thread pooling method and size to the container what is the method to pass such configuration? for example: thread pooling method can be: single thread fixed size ... poolSize is an int vlaue stating the pool size... i am planing to use: java.util.concurrent.Executors best regards ronen Ronen,
I propose an approach different than what you have suggested. Here's my proposal for dealing with this issue:
The rosgi provider currently uses an IExecutor instance to run both synchronous (callSync) and asynchronous (callAsync) remote method invocations. For details, see:
org.eclipse.ecf.internal.provider.r_osgi.RemoteServiceImpl.callSync
and
org.eclipse.ecf.internal.provider.r_osgi.RemoteServiceImpl.callAsync
IExecutor is an interface declared in the Equinox concurrent API...i.e. org.eclipse.equinox.concurrent bundle, org.eclipse.equinox.concurrent.future is the package where IExecutor lives.
Currently, the IExecutor instance for sync and async invocation is gotten by creating a new ThreadsExecutor for each and every sync (and async) invocation. This is indeed wasteful, as ThreadsExecutor creates a new thread for every invocation (as you have discovered).
Rather than build in a compile-time dependency on java5's concurrent API and the thread pooling that it supports...I would rather that the ECF remote service api...and the rosgi provider remain able to run on jre 1.4...instead I propose that we use the OSGI service registry to allow alternative IExecutor implementations to be read at call invoke time from the OSGi service registry...rather than creating a new ThreadsExecutor every time as we do now. That way, people (like yourself...and others) could define *alternative* IExecutor implementations very easily, and simply register them as OSGi services...and then at call invoke time the r-osgi provider would look into the OSGi service registry, use any registered IExecutors...and if none are registered simply fall back to using the ThreadsExecutor as the default.
Note this would let you create an IExecutor based upon the java 1.5 concurrent API if you wish, and have *it* used rather than the ThreadsExecutor. It would also allow people to use the JobsExecutor (based upon the Equinox jobs API...which also has thread pooling), the ImmediateExector (which uses the calling thread to execute IProgressRunnables immediately) or some other IExecutor implementation of their choosing.
For example, to register a new IExecutor for use in the rosgi provider for synchronous invocation, it would be a call like this:
Dictionary props = new Properties();
props.put("org.eclipse.ecf.provider.r_osgi.consumerExecutor","sync");
bundleContext.registerService(IExecutor.class.getName(),new MyExecutor(), props);
Then, in the rosgi provider code would be a call to lookup any synchronous IExecutors from the OSGi service registry...and use it...if any are defined with the property matching "org.eclipse.ecf.provider.r_osgi.consumerExecutor=sync". If none are defined, then it would simply fall back to using the ThreadsExecutor.
I think this would be a much better way to generalize the rosgi provider, as it would then allow whatever alternative thread pooling mechanisms people wanted to use...without putting any compile time dependency on the concurrent API...and allowing a variety of already-implemented alternatives (JobsExecutor, ImmediateExecutor, ThreadsExecutor) to be reused (for example, the JobsExecutor already does thread pooling).
I've already implemented and tested this strategy for the rosgi provider, and will attach the source changes to this bug for examination in another posting.
Downgrading importance...I know it's important to you Ronen, and it will be addressed (possibly as soon as today), but it doesn't rise to the level of critical. Created attachment 192485 [details] zip file with source for rosgi RemoteServiceImpl class and supporting Activator class This is a zip with the source code for the modified rosgi RemoteServiceImpl and supporting Activator class. This implements the IExecutor OSGi service lookup strategy described in comment 2. With approval, I will release this fix to master as soon as a couple of +1s appear. Changing to new target milestone 3.5.1 (In reply to comment #2) Yes, this is the way to go. +1 for Scott's approach. +1 for scott approach i have also tested the patch and it fixed the problem Ok...I've release the strategy outline in comment 2 to master. This fix requires some more documentation, however...i.e. to document the ability to register instances of IExecutor in order to customize the sync and async executors for the rosgi provider (by registering custom instances of IExecutor with the appropriate property value set). Ronen...in return for this prompt fix, I would like to request that you contribute a wiki page addition to document this new aspect of the rosgi provider...with a simple example or two that shows it's usage. Would you be willing to do this? It would be most appreciated...and an excellent first contribution to the ECF community. One other thing...I think the documentation should include a warning of one side-effect introduced by this fix: since the ImmediateExecutor is now used for synchronous rosgi invocation, it means that remote calls that block forever will *not* timeout. Previously, since each synchronous invocation was invoked by a separate consumer-side thread, the remote call would timeout. Now, with the ImmediateExecutor as the default, the invocation can potentially block forever. I'm still sort of debating whether this should be the default, or if the ThreadsExecutor should be the default (as it was before this bug fix). If ThreadsExecutor was the default, it would require Ronen (and others that were concerned about the scaling consequences of the ThreadsExecutor) to use the OSGi service mechanism described above to set to the sync executor to ImmediateExecutor. In either case (the default is ImmediateExecutor or ThreadsExecutor) this should also be clearly documented by the wiki page. Would this wiki page be something you would be willing to provide, Ronen? I'll keep this bug open until we decide what to do WRT the documentation question. sure i will be happy to provide such wiki page Resolving as fixed. Ronen...would there be any chance that you could prepare a short wiki page describing this for ECF 3.5.1? (We are planning release of 3.5.1 for May 19...i.e. prior to Indigo). |
Build Identifier: eclipse 3.6.2 SDK - ECF 3.5 executing the sample code: org.eclipse.ecf.examples.remoteservices.hello.consumer.rs org.eclipse.ecf.examples.remoteservices.hello.host.rs the consumer code was enhanced: // 6. Finally...call the proxy long start, end, total; start = System.currentTimeMillis(); String response=null; long count = 8000; for (int i = 0; i<count;i++) response = proxy.sayHello("RemoteService Consumer"); end = System.currentTimeMillis(); total = end - start; long avg = total/count; System.out.println("total=["+total+"][avg=" +(avg) +"]"+ response); error recived: An internal error has occurred. unable to create new native thread Reproducible: Always Steps to Reproduce: 1. invoke the consumer 8000 times in close loop 2. 3.