logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matt Sicker <boa...@gmail.com>
Subject Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
Date Tue, 29 Apr 2014 03:49:16 GMT
I'll keep it in mind not to do pointless lock changes. Doing a
synchronized(someObject) is perfectly fine, too.


On 28 April 2014 22:21, Ralph Goers <rgoers@apache.org> wrote:

> I think you are still missing the point.  Whether I agree with the
> arguments or not, I still don't think it is worth the time to change stuff
> like this that is already working just because it is considered a better
> style.  You have a better argument with the synchronization stuff because
> it could possibly perform better (or not - there seems to be some
> discussion on that), but I am not in favor of making changes just because
> someone feels like redecorating the room.  That said, you already made the
> change and I don't think it is worth the effort to change it back either.
>
> Ralph
>
> On Apr 28, 2014, at 7:19 PM, Matt Sicker <boards@gmail.com> wrote:
>
> Here, I found a debate over what I was trying to convey:
>
>
> http://stackoverflow.com/questions/541487/implements-runnable-vs-extends-thread
>
>
> On 28 April 2014 12:22, Ralph Goers <ralph.goers@dslextreme.com> wrote:
>
>> Preferring Executors to Threads really has nothing to do with the change
>> below since there isn’t an Executor in sight in the old or the new.  If you
>> want a pool of Threads then item 68 applies. For a single thread creating a
>> Thread with a run method is OK. So is the way you did it - it is just more
>> verbose.
>>
>> Ralph
>>
>>
>> On Apr 28, 2014, at 9:33 AM, Matt Sicker <boards@gmail.com> wrote:
>>
>> It's based on Item 68 from Effective Java (prefer executors and tasks to
>> threads), and those use Runnable and Callable<T> instead of Thread. Plus
>> it's simpler to implement Runnable and construct a Thread from it if
>> necessary. Overall, it's better to use the Executors class to spawn off
>> threads.
>>
>>
>> On 27 April 2014 23:22, Ralph Goers <ralph.goers@dslextreme.com> wrote:
>>
>>> I wondered the same thing myself. I think Matt stated he preferred
>>> Runnable to extending Thread in the past, but I really don’t like changes
>>> like this just for the sake of someone’s personal preference.  While we do
>>> have sort of an unwritten list of coding guidelines this one isn’t on that
>>> list (at least, not yet).
>>>
>>> Ralph
>>>
>>>
>>> On Apr 27, 2014, at 8:22 PM, Remko Popma <remko.popma@gmail.com> wrote:
>>>
>>> > Hi Matt,
>>> > I don't mind this change, but why do you think this is better? This is
>>> all private internal & the previous code was shorter...
>>> >
>>> > Remko
>>> >
>>> > Sent from my iPhone
>>> >
>>> >> On 2014/04/28, at 3:31, mattsicker@apache.org wrote:
>>> >>
>>> >> Author: mattsicker
>>> >> Date: Sun Apr 27 18:31:20 2014
>>> >> New Revision: 1590447
>>> >>
>>> >> URL: http://svn.apache.org/r1590447
>>> >> Log:
>>> >> Convert anonymous thread to runnable.
>>> >>
>>> >> Modified:
>>> >>
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
>>> >>
>>> >> Modified:
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
>>> >> URL:
>>> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java?rev=1590447&r1=1590446&r2=1590447&view=diff
>>> >>
>>> ==============================================================================
>>> >> ---
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
>>> (original)
>>> >> +++
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
>>> Sun Apr 27 18:31:20 2014
>>> >> @@ -31,20 +31,20 @@ public final class CachedClock implement
>>> >>    private static CachedClock instance = new CachedClock();
>>> >>    private volatile long millis = System.currentTimeMillis();
>>> >>    private volatile short count = 0;
>>> >> -    private final Thread updater = new Thread("Clock Updater
>>> Thread") {
>>> >> -        @Override
>>> >> -        public void run() {
>>> >> -            while (true) {
>>> >> -                final long time = System.currentTimeMillis();
>>> >> -                millis = time;
>>> >> -
>>> >> -                // avoid explicit dependency on sun.misc.Util
>>> >> -                LockSupport.parkNanos(1000 * 1000);
>>> >> -            }
>>> >> -        }
>>> >> -    };
>>> >>
>>> >>    private CachedClock() {
>>> >> +        final Thread updater = new Thread(new Runnable() {
>>> >> +            @Override
>>> >> +            public void run() {
>>> >> +                while (true) {
>>> >> +                    final long time = System.currentTimeMillis();
>>> >> +                    millis = time;
>>> >> +
>>> >> +                    // avoid explicit dependency on sun.misc.Util
>>> >> +                    LockSupport.parkNanos(1000 * 1000);
>>> >> +                }
>>> >> +            }
>>> >> +        }, "Clock Updater Thread");
>>> >>        updater.setDaemon(true);
>>> >>        updater.start();
>>> >>    }
>>> >>
>>> >>
>>> >
>>> > ---------------------------------------------------------------------
>>> > To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>>> > For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>> >
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>
>>>
>>
>>
>> --
>> Matt Sicker <boards@gmail.com>
>>
>>
>>
>
>
> --
> Matt Sicker <boards@gmail.com>
>
>


-- 
Matt Sicker <boards@gmail.com>

Mime
View raw message