You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Ralph Goers <ra...@dslextreme.com> on 2014/04/28 09:07:00 UTC
Log4j Project Guidelines
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-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org
Re: Log4j Project Guidelines
Posted by Matt Sicker <bo...@gmail.com>.
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 <ra...@dslextreme.com> wrote:
> 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 <re...@gmail.com> wrote:
>
> 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 <ga...@gmail.com>wrote:
>
>> I'm pretty sure Eclipse does that.
>>
>> Gary
>>
>>
>> On Wed, Apr 30, 2014 at 9:50 AM, Matt Sicker <bo...@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 <re...@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 <bo...@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 <ga...@gmail.com> wrote:
>>>>>
>>>>>> I like the KISS approach for now as well.
>>>>>>
>>>>>> Gary
>>>>>>
>>>>>>
>>>>>> On Tue, Apr 29, 2014 at 9:45 PM, Remko Popma <re...@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 <bo...@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 <jh...@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 (boards@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-unsubscribe@logging.apache.org
>>>>>>>>> > >>>> For additional commands, e-mail:
>>>>>>>>> log4j-dev-help@logging.apache.org
>>>>>>>>> > >>>>
>>>>>>>>> > >>>>
>>>>>>>>> > >>>
>>>>>>>>> > >>>
>>>>>>>>> > >>> --
>>>>>>>>> > >>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@logging.apache.org
>>>>>>>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Matt Sicker <bo...@gmail.com>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> Matt Sicker <bo...@gmail.com>
>>>
>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
Re: Log4j Project Guidelines
Posted by Ralph Goers <ra...@dslextreme.com>.
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 <re...@gmail.com> wrote:
> 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 <ga...@gmail.com> wrote:
> I'm pretty sure Eclipse does that.
>
> Gary
>
>
> On Wed, Apr 30, 2014 at 9:50 AM, Matt Sicker <bo...@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 <re...@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 <bo...@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 <ga...@gmail.com> wrote:
> I like the KISS approach for now as well.
>
> Gary
>
>
> On Tue, Apr 29, 2014 at 9:45 PM, Remko Popma <re...@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 <bo...@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 <jh...@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 (boards@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-unsubscribe@logging.apache.org
> > >>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
> > >>>>
> > >>>>
> > >>>
> > >>>
> > >>> --
> > >>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
>
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@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
>
Re: Log4j Project Guidelines
Posted by Remko Popma <re...@gmail.com>.
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 <ga...@gmail.com>wrote:
> I'm pretty sure Eclipse does that.
>
> Gary
>
>
> On Wed, Apr 30, 2014 at 9:50 AM, Matt Sicker <bo...@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 <re...@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 <bo...@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 <ga...@gmail.com> wrote:
>>>>
>>>>> I like the KISS approach for now as well.
>>>>>
>>>>> Gary
>>>>>
>>>>>
>>>>> On Tue, Apr 29, 2014 at 9:45 PM, Remko Popma <re...@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 <bo...@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 <jh...@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 (boards@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-unsubscribe@logging.apache.org
>>>>>>>> > >>>> For additional commands, e-mail:
>>>>>>>> log4j-dev-help@logging.apache.org
>>>>>>>> > >>>>
>>>>>>>> > >>>>
>>>>>>>> > >>>
>>>>>>>> > >>>
>>>>>>>> > >>> --
>>>>>>>> > >>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@logging.apache.org
>>>>>>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Matt Sicker <bo...@gmail.com>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
>>>>
>>>
>>>
>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@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
>
Re: Log4j Project Guidelines
Posted by Gary Gregory <ga...@gmail.com>.
I'm pretty sure Eclipse does that.
Gary
On Wed, Apr 30, 2014 at 9:50 AM, Matt Sicker <bo...@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 <re...@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 <bo...@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 <ga...@gmail.com> wrote:
>>>
>>>> I like the KISS approach for now as well.
>>>>
>>>> Gary
>>>>
>>>>
>>>> On Tue, Apr 29, 2014 at 9:45 PM, Remko Popma <re...@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 <bo...@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 <jh...@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 (boards@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-unsubscribe@logging.apache.org
>>>>>>> > >>>> For additional commands, e-mail:
>>>>>>> log4j-dev-help@logging.apache.org
>>>>>>> > >>>>
>>>>>>> > >>>>
>>>>>>> > >>>
>>>>>>> > >>>
>>>>>>> > >>> --
>>>>>>> > >>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@logging.apache.org
>>>>>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Matt Sicker <bo...@gmail.com>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
>>>
>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
--
E-Mail: garydgregory@gmail.com | ggregory@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
Re: Log4j Project Guidelines
Posted by Matt Sicker <bo...@gmail.com>.
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 <re...@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 <bo...@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 <ga...@gmail.com> wrote:
>>
>>> I like the KISS approach for now as well.
>>>
>>> Gary
>>>
>>>
>>> On Tue, Apr 29, 2014 at 9:45 PM, Remko Popma <re...@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 <bo...@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 <jh...@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 (boards@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-unsubscribe@logging.apache.org
>>>>>> > >>>> For additional commands, e-mail:
>>>>>> log4j-dev-help@logging.apache.org
>>>>>> > >>>>
>>>>>> > >>>>
>>>>>> > >>>
>>>>>> > >>>
>>>>>> > >>> --
>>>>>> > >>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@logging.apache.org
>>>>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Matt Sicker <bo...@gmail.com>
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
>>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: Log4j Project Guidelines
Posted by Remko Popma <re...@gmail.com>.
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 <bo...@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 <ga...@gmail.com> wrote:
>
>> I like the KISS approach for now as well.
>>
>> Gary
>>
>>
>> On Tue, Apr 29, 2014 at 9:45 PM, Remko Popma <re...@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 <bo...@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 <jh...@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 (boards@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-unsubscribe@logging.apache.org
>>>>> > >>>> For additional commands, e-mail:
>>>>> log4j-dev-help@logging.apache.org
>>>>> > >>>>
>>>>> > >>>>
>>>>> > >>>
>>>>> > >>>
>>>>> > >>> --
>>>>> > >>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@logging.apache.org
>>>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Matt Sicker <bo...@gmail.com>
>>>>
>>>
>>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
>
Re: Log4j Project Guidelines
Posted by Matt Sicker <bo...@gmail.com>.
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 <ga...@gmail.com> wrote:
> I like the KISS approach for now as well.
>
> Gary
>
>
> On Tue, Apr 29, 2014 at 9:45 PM, Remko Popma <re...@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 <bo...@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 <jh...@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 (boards@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-unsubscribe@logging.apache.org
>>>> > >>>> For additional commands, e-mail:
>>>> log4j-dev-help@logging.apache.org
>>>> > >>>>
>>>> > >>>>
>>>> > >>>
>>>> > >>>
>>>> > >>> --
>>>> > >>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@logging.apache.org
>>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>>
>>>>
>>>
>>>
>>> --
>>> Matt Sicker <bo...@gmail.com>
>>>
>>
>>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
Re: Log4j Project Guidelines
Posted by Gary Gregory <ga...@gmail.com>.
I like the KISS approach for now as well.
Gary
On Tue, Apr 29, 2014 at 9:45 PM, Remko Popma <re...@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 <bo...@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 <jh...@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 (boards@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-unsubscribe@logging.apache.org
>>> > >>>> For additional commands, e-mail:
>>> log4j-dev-help@logging.apache.org
>>> > >>>>
>>> > >>>>
>>> > >>>
>>> > >>>
>>> > >>> --
>>> > >>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@logging.apache.org
>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>
>>>
>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>>
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@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
Re: Log4j Project Guidelines
Posted by Remko Popma <re...@gmail.com>.
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 <bo...@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 <jh...@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 (boards@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-unsubscribe@logging.apache.org
>> > >>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>> > >>>>
>> > >>>>
>> > >>>
>> > >>>
>> > >>> --
>> > >>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@logging.apache.org
>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
Re: Log4j Project Guidelines
Posted by Matt Sicker <bo...@gmail.com>.
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 <jh...@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 (boards@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-unsubscribe@logging.apache.org
> > >>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
> > >>>>
> > >>>>
> > >>>
> > >>>
> > >>> --
> > >>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>
--
Matt Sicker <bo...@gmail.com>
Re: Log4j Project Guidelines
Posted by Jörn Huxhorn <jh...@googlemail.com>.
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 (boards@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-unsubscribe@logging.apache.org
> >>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
> >>>>
> >>>>
> >>>
> >>>
> >>> --
> >>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org
Re: Log4j Project Guidelines
Posted by Matt Sicker <bo...@gmail.com>.
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 <ra...@dslextreme.com> 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 <bo...@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 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 <ra...@dslextreme.com> 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 <bo...@gmail.com> 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 <ga...@gmail.com> 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 <ralph.goers@dslextreme.com
>>> > 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-unsubscribe@logging.apache.org
>>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>>
>>>>
>>>
>>>
>>> --
>>> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
>>
>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: Log4j Project Guidelines
Posted by Ralph Goers <ra...@dslextreme.com>.
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 <bo...@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 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 <ra...@dslextreme.com> 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 <bo...@gmail.com> 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 <ga...@gmail.com> 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 <ra...@dslextreme.com> 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-unsubscribe@logging.apache.org
>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>
>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
>
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
Re: Log4j Project Guidelines
Posted by Matt Sicker <bo...@gmail.com>.
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 <ra...@dslextreme.com> 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 <bo...@gmail.com> 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 <ga...@gmail.com> 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 <ra...@dslextreme.com>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-unsubscribe@logging.apache.org
>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>
>>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: Log4j Project Guidelines
Posted by Ralph Goers <ra...@dslextreme.com>.
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 <bo...@gmail.com> 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 <ga...@gmail.com> 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 <ra...@dslextreme.com> 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-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
Re: Log4j Project Guidelines
Posted by Matt Sicker <bo...@gmail.com>.
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 <ga...@gmail.com> 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 <ra...@dslextreme.com>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-unsubscribe@logging.apache.org
>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>
>>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
Re: Log4j Project Guidelines
Posted by Gary Gregory <ga...@gmail.com>.
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 <ra...@dslextreme.com>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-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@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
Re: Log4j Project Guidelines
Posted by Paul Benedict <pb...@apache.org>.
I recommend this gets reposted to the wiki or the site docs.
On Mon, Apr 28, 2014 at 2:07 AM, Ralph Goers <ra...@dslextreme.com>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-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>
--
Cheers,
Paul