You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ma...@apache.org on 2014/04/27 20:31:21 UTC
svn commit: r1590447 -
/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
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();
}
Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
Posted by Matt Sicker <bo...@gmail.com>.
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 <rg...@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 <bo...@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 <ra...@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 <bo...@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 <ra...@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 <re...@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 <bo...@gmail.com>
>>
>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
Posted by Ralph Goers <rg...@apache.org>.
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 <bo...@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 <ra...@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 <bo...@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 <ra...@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 <re...@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 <bo...@gmail.com>
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
Posted by Matt Sicker <bo...@gmail.com>.
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 <ra...@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 <bo...@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 <ra...@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 <re...@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 <bo...@gmail.com>
>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
Posted by Ralph Goers <ra...@dslextreme.com>.
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 <bo...@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 <ra...@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 <re...@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 <bo...@gmail.com>
Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
Posted by Matt Sicker <bo...@gmail.com>.
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 <ra...@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 <re...@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 <bo...@gmail.com>
Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
Posted by Ralph Goers <ra...@dslextreme.com>.
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 <re...@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
Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java
Posted by Remko Popma <re...@gmail.com>.
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