Re: Log4j Project Guidelines

2014-04-30 Thread Matt Sicker
So it's a good idea to set up your IDE to warn if you override a synchronized method with an unsynchronized method? I remember seeing that one somewhere. On 30 April 2014 00:35, Remko Popma remko.po...@gmail.com wrote: Agreed that unless the synchronization is part of the public API it is

Re: Log4j Project Guidelines

2014-04-30 Thread Gary Gregory
I'm pretty sure Eclipse does that. Gary On Wed, Apr 30, 2014 at 9:50 AM, Matt Sicker boa...@gmail.com wrote: So it's a good idea to set up your IDE to warn if you override a synchronized method with an unsynchronized method? I remember seeing that one somewhere. On 30 April 2014 00:35,

Re: Log4j Project Guidelines

2014-04-30 Thread Remko Popma
Yes, overriding a synchronized method with an unsynchronized method sounds dangerous... Warnings would probably be helpful. On Wed, Apr 30, 2014 at 11:13 PM, Gary Gregory garydgreg...@gmail.comwrote: I'm pretty sure Eclipse does that. Gary On Wed, Apr 30, 2014 at 9:50 AM, Matt Sicker

synchronized vs. Lock (was: Log4j Project Guidelines)

2014-04-30 Thread Jörn Huxhorn
Did you try https://gist.github.com/huxi/81c152a672d1b39aed64 with different delays? (If you find anything wrong with that code, please let me know.) The output on my system with 0ms, 1ms and 10ms delays looks like this:  http://pastebin.com/za07QmUU Unfairness increases with the amount of time

Re: Log4j Project Guidelines

2014-04-30 Thread Ralph Goers
Actually, I believe there was a case where I did that because the method in the subclass used java.util.concurrent instead of requiring method synchronization. Ralph On Apr 30, 2014, at 7:19 AM, Remko Popma remko.po...@gmail.com wrote: Yes, overriding a synchronized method with an

Re: Log4j Project Guidelines

2014-04-30 Thread Matt Sicker
There are legitimate reasons to do that as long as you maintain thread-safety such as the case Ralph mentioned. Though that would probably require full control of the API from top to bottom-ish. On 30 April 2014 10:22, Ralph Goers ralph.go...@dslextreme.com wrote: Actually, I believe there was

Re: Log4j Project Guidelines

2014-04-29 Thread Jörn Huxhorn
Please keep in mind that synchronized is not fair. http://sourceforge.net/apps/trac/lilith/wiki/SynchronizedVsFairLock Yes, a fair ReentrantLock is way slower than an unfair one… but if starvation is caused by a logging framework then this is a serious issue in my opinion. Joern On 29. April

Re: Log4j Project Guidelines

2014-04-29 Thread Matt Sicker
Should we be using a fair lock? That's usually a lot slower than a typical one, but if it's more proper behavior, then it would make sense to go that route. On 29 April 2014 14:49, Jörn Huxhorn jhuxh...@googlemail.com wrote: Please keep in mind that synchronized is not fair.

Re: Log4j Project Guidelines

2014-04-29 Thread Gary Gregory
I like the KISS approach for now as well. Gary On Tue, Apr 29, 2014 at 9:45 PM, Remko Popma remko.po...@gmail.com wrote: This should be a separate thread, but anyway... I would oppose using fair locks. Not only will throughput be much less, the benefits are debatable, since fairness of

Re: Log4j Project Guidelines

2014-04-29 Thread Matt Sicker
If you want more than one condition monitor, it's also better to use Lock. I'll definitely agree on the simplicity of synchronizing on an internal Object instance, though. Locking on this, however, is generally a bad idea because any other code can synchronize on the object as well by accident and

Re: Log4j Project Guidelines

2014-04-29 Thread Remko Popma
Agreed that unless the synchronization is part of the public API it is better to lock on an internal object (e.g. private Object lock = new Object(); ). On the other hand, OutputStreamManager and subclasses have some synchronized public/protected methods and in this case the synchronization on

Log4j Project Guidelines

2014-04-28 Thread Ralph Goers
I think we need to develop and post some development “guidelines”, “best practices” or whatever you want to call it for Log4j 2. Here are some of the things I would definitely want on it. 1. Error on the side of caution. If you don’t understand it, don’t touch it and ask on the list. If you

Re: Log4j Project Guidelines

2014-04-28 Thread Paul Benedict
I recommend this gets reposted to the wiki or the site docs. On Mon, Apr 28, 2014 at 2:07 AM, Ralph Goers ralph.go...@dslextreme.comwrote: I think we need to develop and post some development “guidelines”, “best practices” or whatever you want to call it for Log4j 2. Here are some of the

Re: Log4j Project Guidelines

2014-04-28 Thread Gary Gregory
Perhaps if one breaks the build, it should be polite to revert that last commit... Gary On Mon, Apr 28, 2014 at 3:07 AM, Ralph Goers ralph.go...@dslextreme.comwrote: I think we need to develop and post some development “guidelines”, “best practices” or whatever you want to call it for Log4j

Re: Log4j Project Guidelines

2014-04-28 Thread Matt Sicker
What about style things covered by Effective Java? These are pretty much all good ideas. Also, how about some guidelines on concurrency? I'd recommend not using the old concurrency stuff and instead using java.util.concurrent.* for more effective concurrency. This doesn't include the Thread vs

Re: Log4j Project Guidelines

2014-04-28 Thread Ralph Goers
Yes, guidelines called out by Effective Java are appropriate when they apply. As for concurrency, “New” isn’t always better than old. In a few places you changed synchronized(object) to use a Lock instead. There is little to no value in doing that and makes the code look a little more

Re: Log4j Project Guidelines

2014-04-28 Thread Matt Sicker
In that case, Item 69: prefer concurrency utilities to wait and notify. Sounds like we can just use a plain Object instance to lock (which is pretty much equivalent to using ReentrantLock) when doing normal locks, but instead of using .notifyAll() and .wait(), we should use the Condition interface

Re: Log4j Project Guidelines

2014-04-28 Thread Ralph Goers
Why are they not appropriate lock objects? Start a discussion before just changing them. Ralph On Apr 28, 2014, at 10:40 AM, Matt Sicker boa...@gmail.com wrote: In that case, Item 69: prefer concurrency utilities to wait and notify. Sounds like we can just use a plain Object instance to

Re: Log4j Project Guidelines

2014-04-28 Thread Matt Sicker
I'll delegate my arguments to the SO post about it: http://stackoverflow.com/questions/442564/avoid-synchronizedthis-in-java In short, it's defensive programming. It's safer to synchronize on an internal object than on this. Plus, it aids in concurrency throughput instead of just using a