On Thu, 2004-09-23 at 01:59, Ceki Gülcü wrote:
> Some would say heavy-handed, others would say simple and robust. Given > the history of this discussion (see bug report 24159), I have a strong > bias against modifying the existing synchronization code in > AppenderSkeleton. The problem in 24159 is just too contrived to be > taken seriously.
I feel a bit frustrated because I've been bitten in the ass a few times encountering this "contrived" problem and I haven't been taken very seriously pointing this out. As I have tried to explain, this is something that has happened before on servers at this company, and it's something we'd like to have fixed.
You are right to observe that bug 24159 has not been seriously. I am sorry if you feel frustrated by the response to your inquiry. However, I think bug 24159 can be fixed without modifying log4j code, as described below.
In order to fix bug 24159, would it be possible for you to not log from within a synchronized resource which is also used from within rendering code? In your example, would it be possible for you not to log from within the setState() method? The setState method is synchronized on a resource, i.e. the State object instance, which is also accessed in a synchronized manner from the toString() method. This access pattern is the reason for the for the deadlock you are observing. How difficult is it to avoid this pattern?
I thought in the quest for helping fix this bug we could improve concurrency and multi-threading. Even if you think this bug is dumb, wouldn't you like to improve in this area?
I appreciate this discussion and especially your efforts in improving concurrency in log4j code. However, since writing an event to disk or any other device takes over 90% of the time required to complete a logging operation, there is little to be gained in improving concurrency outside those 90%. Note that this statement is true only if the event is written to disk, if the event is disabled it is a completely different ball game.
Now, bug 24159 occurs when the event is enabled, i.e. written to disk or any other output device. Given the overwhelming weight of the code for formatting and writing onto the output device (over 90% of cost), I really don't think any noticeable performance gains could be obtained by increasing concurrency in the code exercised by the doAppend() method.
As far as I can tell, the only advantage obtained in changing the concurrency model would be in avoiding the deadlock situations, e.g. bug 24159. As mentioned earlier, bug 24159 could probably be solved without modifying the existing log4j concurrently model.
I sent out some changes that will maintain compatibility and fix this issue, have you seen them?\
Yes, I have. As you know, I have also spend considerable amount of time understanding and testing bug 24159.
-- Ceki G�lc�
For log4j documentation consider "The complete log4j manual"
ISBN: 2970036908 http://www.qos.ch/shop/products/clm_t.jsp
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
