[
https://issues.apache.org/jira/browse/ETCH-258?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13591537#comment-13591537
]
Paul Turner commented on ETCH-258:
----------------------------------
Summary
I have not changed things just to change them. The changes I have made to
convert to using util.concurrent classes are mainly for reasons such as ability
to recycle threads.
----------
org.apache.etch.util.TodoManager
Motivation for change:
TodoManager is a threadpool with a capped queue capacity. Methods to schedule
the execution of a Todo are synchronized,.
System performance will degrade as too many requests are being created for
the threadpool and when it cannot service them. This means that method calls
will accumulate whilst waiting for the instance scoped lock and will use memory
beyond the capacity of the work queue. In effect, waiting synchronized method
calls is a hidden and unbounded queue.
Change:
An approach used in the java.util.concurrent threadpool framework to deal with
this degradation is to force calling threads to carry out the execution of
their own items scheduled on the threadpool.
This is achieved by using a CallerRunsPolicy as the RejectedExecutionHandler in
ThreadPoolExecutor.
This has 2 beneficial effects:
1: The producer threads ability to saturate the threadpool is limited because
more more of their time is spent servicing requests rather than producing them.
2: The amount of work to be done, saturation of threadpool and memory used in
calls awaiting instance lock is reduced.
I have changed the TodoManager class to use standard ExecutorService framework
for the reasons stated above.
Consequence of Change:
A feature of the original TodoManager is lost. This is:
Ability to sleep while waiting for work queue capacity. (As discussed
previously)
This was configured by the entryDelay in the constructor of TodoManager. The
old constructor remains for backwards-compatibility but is @Deprecated.
All public methods remain with the same contract.
Unit test added.
----------
org.apache.etch.util.AlarmManager
Motivation for change:
Adding alarms does not take advanatage of Java standard api classes.
Change:
I have changed the AlarmManager class to use a ScheduledExecutor. All public
methods remain with the same contract.
----------
org.apache.etch.util.IdGenerator
Motivation for change:
Use standard Java apis for atomic incrementing number functionality
Change:
Instance level synchronization no longer necessary, An AtomicLong now takes
care of atomic incrementor functionality. All public methods remain with the
same contract. Unit test added.
----------
org.apache.etch.bindings.java.support.FreePool
Motivation for change:
This is a threadpool and would benefit from the use of standard Java api
classes.
Change:
Threadpool functionality now provided by Executors framework. A
SynchrouousQueue is used to ensure a queue-free implementation rejecting
runnables with an exception where necessary.
Overall effect is that threads are recycled by the threadpoolexecutor rather
than being re-created for each task. All public methods remain with the same
contract.
----------
org.apache.etch.bindings.java.support.TestFreePool
TestFreePool.java
Motivation for change:
Unit tests were unpredictably failing due to a race condition in that the
asynchronous PoolRunnable execution may not have completed before the unit test
Asserts are called.
Change:
The main thread of the unit test is now blocked by a CountDownLatch until all
the asychronous PoolRunnables complete. I also added another test to make sure
the SynchronousQueue used as the threadpool BlockingQueue calles the
RejectedExecutionHandler as expected. (Basically a test for throwing an
Exception when pool full -> probably a repeat of existing tests).
----------
Remaining Work
I would like to make the Monitor support concurrent waitUntil* calls. This is
because the current implementation blocks on e.g. waitUntilEq and it is not
possible to lock acquire in a fair way using the synchronized keyword. Out of
2 synchronized calls to waitUntilEq one will obtain the lock and the other will
wait. After the first method call has released the lock a call to set() could
be executed before the second call to waitUntilEq(...) obtains the lock. This
means that when the second call to waitUntilEq eventually acquires the lock it
could see a different value that was present when the method was invoked. This
is very difficult because coordination between waitUntil*(...) and set(...)
require the use of a lock but once this lock has been acquired and the blocking
call made to wait for changes, it is not possible to release the lock to enable
other calls to set(..) or waitUntil*(...).
A test case for this is as follows.
A monitor is set up with value = 1 and then to calls are made in separate
threads to waitUntilEq. One waiting for 2, and later another waiting for 1.
Expected result: The call to waitUntilEq(2) will block indefinitely, the call
to waitUntilEq(1) should return immediately.
Observed result: Both calls to waitUntilEq(...) block indefinitely.
I have added this test case to TestMonitor but without the @Test annotation so
as not to affect the build.
I would also like to refactor the CircularQueue to use a BlockingDeque as the
internal double-ended queue.
> Switch to using util.concurrent instead of pre Java 5 threading constructs
> --------------------------------------------------------------------------
>
> Key: ETCH-258
> URL: https://issues.apache.org/jira/browse/ETCH-258
> Project: Etch
> Issue Type: Improvement
> Components: binding-java, general
> Reporter: Paul Turner
> Priority: Minor
> Fix For: 1.4
>
> Attachments: etch-20130301.patch
>
> Original Estimate: 168h
> Remaining Estimate: 168h
>
> thread creation is quite expensive and so a new thread per unit of work is
> also expensive, i propose to use util.concurrent threadpools in the java
> binding sub-project and enhance unit tests e.g. with countdown latches to
> ensure competing test threads start simeltanously and semaphore to throttle
> access to running units of work.
> affects FreePool, TodoManager and associated tests and possibly more classes
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
