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 cause weird problems.
On 29 April 2014 20:51, Gary Gregory <garydgreg...@gmail.com> wrote: > 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 > -- Matt Sicker <boa...@gmail.com>
