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, Remko Popma <remko.po...@gmail.com> wrote: > >> 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 "this" is part of the public API. >> >> >> >> On Wed, Apr 30, 2014 at 1:24 PM, Matt Sicker <boa...@gmail.com> wrote: >> >>> 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> >>> >> >> > > > -- > 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
