Reputation: 23
I'm writing an application that draws certain shapes.
Before each shape is drawn, some calculations are to be made. Which is why I have another thread to draw already processed items.
To synchronize between the two threads, I used a Semaphore. Before the drawer tries to extract an item from the queue, it will acquire a permit and every time an item is enqueued it will release a permit. Besides that, the enqueue
and dequeue
methods are synchronized. the calculating thread will wait for the drawer to finish if the number of permits isn't zero by the time the calculating thread is done.
EDIT The synchronized block in the calculating thread is to prevent the calculating thread to go on before the drawing thread is done. If it did, it would mess things up.
After profiling, releasing permits takes 8.3%
of the calculating thread's run time. Which is acceptable. The problem is with the drawing thread. The acquire method takes 63% of it's run time.
Besides that, I tried using a blocking queue, but it was even worse.
code:
drawing thread:
private void drawNext(){
QueuedDraw item;
if(drawingStatus.availablePermits()==0)
synchronized(drawingStatus){
if(drawingStatus.availablePermits()==0)
drawingStatus.notify();
}
try {
drawingStatus.acquire();
} catch (InterruptedException ex) {
Logger.getLogger(PolygonDrawer.class.getName())
.log(Level.SEVERE, null, ex);
}
item = queue.dequeue();
fillPoly(item.points, item.color, item.vertices);
}
public final void fillSquare(Point[] points, byte[] color){
queue.enqueue(new QueuedDraw(points, color, SQUARE_VERTICES));
drawingStatus.release();
}
public final void fillTriangle(Point[] points, byte[] color){
queue.enqueue(new QueuedDraw(points, color, TRIANGLE_VERTICES));
drawingStatus.release();
}
@Override
public void run() {
while(true){
drawNext();
}
}
calculating thread:
note: before this block are all the method calls that trigger fillSquare/fillTriangle
if(paintStatus.availablePermits()!=0)
synchronized(paintStatus){
try {
if(paintStatus.availablePermits()!=0)
paintStatus.wait();
} catch (InterruptedException ex) {
Logger.getLogger(Painter.class.getName())
.log(Level.SEVERE, null, ex);
}
}
Upvotes: 2
Views: 1310
Reputation: 1857
Use BlockingQueue.take
and BlockingQueus.put
.
The reason why these or acquire
will show a large share of time spent is (most likely) that those methods block, when nothing is available to take or the Queue is full (if it can be full).
Upvotes: 0
Reputation: 57707
With your current implementation, it seems you are forcing the threads to operate in lockstep. If your queue is thread-safe, such as ArrayBlockingQueue , you don't need the explicit synchronization provided by the semaphore. Once the calculating thread is done updating the item, it can just put it in the queue. The calculating thread can put elements in the queue at the same time the drawing thread fetches them. In principle, you could have multiple calculating threads performing calculations in parallel. The drawing thread uses poll() to fetch items from the queue, which will block the thread if the queue is empty.
This will make the code cleaner, and rely more on established classes. If synchronization overhead is still too high, you can do calculations in batches, and add the batch of calculations to the queue as a single item. The queue type will then be (say) List<QueuedDraw>
.
EDIT: The cost of waiting can be significant (as you've seen) if calculations are relatively quick compared to the cost of blocking, so a blocking queue is not ideal here. Instead, use a queue with a "wait-free" algorithm, such as ConcurrentLinkedQueue. This type of queue doesn't block threads using the scheduler, and instead uses atomic operations to guard access to shared state.
Bear in mind that your drawing thread can only draw as fast as you are calculating. It may make sense that it spends a lot of time waiting for items to become available if they take comparably longer to calculate than to draw. Parallelizing the calculations will help give the drawing thread more work to do.
Upvotes: 3