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 locks does not guarantee > fairness of thread scheduling. So we would always pay a price in > throughput without any guarantee of upside on the latency variance. Not a > good trade-off. > > The solution here is not using different locking mechanisms but not using > locking at all. That is, anyone who is concerned about latency variance (of > which starvation is an extreme case) should be using asynchronous loggers. > The numbers show that with the disruptor (Async Loggers) you get 3 to 5 > *orders of magnitude* less latency. (Yes that means 1000x - 100,000x less > latency.) And this is compared to AsyncAppender. Synchronous logging is not > even in the picture here. > > I do agree that synchronizing on a String should be avoided, because > Strings can be intern()-ed (and in the ConfigurationFactory case it was a > String literal which is intern()ed by default), which means that our lock > object is globally visible and other parts of the system could potentially > synchronize on it and cause deadlock. That was a nice catch. > > However, I'm not convinced that java.util.concurrent.Lock is always better > than the synchronized keyword. It is a more powerful API, and gives more > low-level control, but it comes with more complexity and responsibility, > the most obvious one being you need to manually unlock() it. With the > synchronized keyword I don't need to worry if I or anyone refactoring the > code still correctly unlock()s the lock in a finally clause. It just works. > Nice and simple. It is only when I really need the more powerful API that I > consider using Locks. For example when using separate read and write locks. > Or if I need to use tryLock(). Things that I can't do with the synchronized > keyword. Otherwise I prefer to keep it simple. > > My 2 cents. > > Remko > > > On Wed, Apr 30, 2014 at 5:45 AM, Matt Sicker <boa...@gmail.com> wrote: > >> 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. >>> >>> 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 2014 at 01:05:26, Matt Sicker (boa...@gmail.com) wrote: >>> > 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 synchronized method. >>> > >>> > >>> > On 28 April 2014 12:45, Ralph Goers wrote: >>> > >>> > > 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 wrote: >>> > > >>> > > 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 (which would require using Lock as well). >>> > > >>> > > I agree that using synchronized(object) makes sense when it's all >>> that's >>> > > being done. However, I've been changing instances of >>> synchronized(this) and >>> > > synchronized(foo) where foo is not an appropriate lock object (e.g., >>> a >>> > > string, or a non-final object, things like that). >>> > > >>> > > >>> > > On 28 April 2014 12:28, Ralph Goers wrote: >>> > > >>> > >> 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 >>> cluttered. >>> > >> However, if a ReadWriteLock can be used in place of synchronized >>> that is a >>> > >> real benefit. >>> > >> >>> > >> The point of the guidelines are that when it comes to stuff like >>> this, >>> > >> unless there is a guideline written down that says the current code >>> is >>> > >> wrong discuss it on the list before making a change. >>> > >> >>> > >> Ralph >>> > >> >>> > >> On Apr 28, 2014, at 9:35 AM, Matt Sicker wrote: >>> > >> >>> > >> 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 Runnable >>> thing, >>> > >> so that's still debatable. >>> > >> >>> > >> >>> > >> On 28 April 2014 08:46, Gary Gregory wrote: >>> > >> >>> > >>> 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 > >>> > wrote: >>> > >>> >>> > >>>> 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 think you understand it read >>> it again >>> > >>>> or ask until you are sure you do. Nobody will blame you for asking >>> > >>>> questions. >>> > >>>> 2. Don’t break the build - if there is the slightest chance the >>> change >>> > >>>> you are making could cause unit test failures, run all unit >>> tests. Better >>> > >>>> yet, get in the habit of always running the unit tests before >>> doing the >>> > >>>> commit. >>> > >>>> 3. If the build breaks and you have made recent changes then >>> assume you >>> > >>>> broke it and try to fix it. Although it might not have been >>> something you >>> > >>>> did it will make others feel a lot better than having to fix the >>> mistake >>> > >>>> for you. Everyone makes mistakes. Taking responsibility for them >>> is a good >>> > >>>> thing. >>> > >>>> 4. Don’t change things to match your personal preference - the >>> project >>> > >>>> has style guidelines that are validated with checkstyle, PMD, and >>> other >>> > >>>> tools. If you aren’t fixing a bug, fixing a problem identified by >>> the >>> > >>>> tools, or fixing something specifically called out in these >>> guidelines then >>> > >>>> start a discussion to see if the change is something the project >>> wants >>> > >>>> before starting to work on it. We try to discuss things first and >>> then >>> > >>>> implement the consensus reached in the discussion. >>> > >>>> >>> > >>>> Of course, the actual coding conventions we follow should also be >>> > >>>> spelled out, such as indentation, braces style, import ordering >>> and where >>> > >>>> to use the final keyword. >>> > >>>> >>> > >>>> Ralph >>> > >>>> >>> --------------------------------------------------------------------- >>> > >>>> To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org >>> > >>>> For additional commands, e-mail: >>> log4j-dev-h...@logging.apache.org >>> > >>>> >>> > >>>> >>> > >>> >>> > >>> >>> > >>> -- >>> > >>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org >>> > >>> Java Persistence with Hibernate, Second Edition >>> > >>> JUnit in Action, Second Edition >>> > >>> Spring Batch in Action >>> > >>> Blog: http://garygregory.wordpress.com >>> > >>> Home: http://garygregory.com/ >>> > >>> Tweet! http://twitter.com/GaryGregory >>> > >>> >>> > >> >>> > >> >>> > >> >>> > >> -- >>> > >> Matt Sicker >>> > >> >>> > >> >>> > >> >>> > > >>> > > >>> > > -- >>> > > Matt Sicker >>> > > >>> > > >>> > > >>> > >>> > >>> > -- >>> > Matt Sicker >>> > >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org >>> For additional commands, e-mail: log4j-dev-h...@logging.apache.org >>> >>> >> >> >> -- >> Matt Sicker <boa...@gmail.com> >> > > -- E-Mail: garydgreg...@gmail.com | ggreg...@apache.org Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> Spring Batch in Action <http://www.manning.com/templier/> Blog: http://garygregory.wordpress.com Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
