[PATCH 1/1] Get rid of synchronization in java.util.logging.LogRecord constructor

[David M. Lloyd]

I couldn't think of any situation where there would be.

- DML

On 03/12/2009 06:46 PM, Martin Buchholz wrote:
> This looks fine, as long as there is no dependency of the implementation
> on the two atomic counters being incremented in concert, as seems likely.
> 
> Martin
> 
> On Thu, Mar 12, 2009 at 15:35, David M. Lloyd <david.lloyd at redhat.com> wrote:
>> Switch to atomic ops for the various sequence numbers, as opposed to
>> synchronizing on the class object on every object construction.
>>
>> - DML
>> --
>>
>> diff -r dde3fe2e8164 src/share/classes/java/util/logging/LogRecord.java
>> --- a/src/share/classes/java/util/logging/LogRecord.java        Wed Feb 25
>> 14:32:01 2009 +0000
>> +++ b/src/share/classes/java/util/logging/LogRecord.java        Thu Mar 12
>> 17:12:22 2009 -0500
>> @@ -25,6 +25,8 @@
>>
>>  package java.util.logging;
>>  import java.util.*;
>> +import java.util.concurrent.atomic.AtomicLong;
>> +import java.util.concurrent.atomic.AtomicInteger;
>>  import java.io.*;
>>
>>  /**
>> @@ -64,9 +66,9 @@
>>  */
>>
>>  public class LogRecord implements java.io.Serializable {
>> -    private static long globalSequenceNumber;
>> -    private static int nextThreadId=10;
>> -    private static ThreadLocal<Integer> threadIds = new
>> ThreadLocal<Integer>();
>> +    private static final AtomicLong globalSequenceNumber = new
>> AtomicLong();
>> +    private static final AtomicInteger nextThreadId = new
>> AtomicInteger(10);
>> +    private static final ThreadLocal<Integer> threadIds = new
>> ThreadLocal<Integer>();
>>
>>     /**
>>      * @serial Logging message level
>> @@ -144,15 +146,13 @@
>>         this.level = level;
>>         message = msg;
>>         // Assign a thread ID and a unique sequence number.
>> -        synchronized (LogRecord.class) {
>> -            sequenceNumber = globalSequenceNumber++;
>> -            Integer id = threadIds.get();
>> -            if (id == null) {
>> -                id = new Integer(nextThreadId++);
>> -                threadIds.set(id);
>> -            }
>> -            threadID = id.intValue();
>> +        sequenceNumber = globalSequenceNumber.getAndIncrement();
>> +        Integer id = threadIds.get();
>> +        if (id == null) {
>> +            id = Integer.valueOf(nextThreadId.getAndIncrement());
>> +            threadIds.set(id);
>>         }
>> +        threadID = id.intValue();
>>         millis = System.currentTimeMillis();
>>         needToInferCaller = true;
>>    }
>>
>> diff -r dde3fe2e8164 src/share/classes/java/util/logging/LogRecord.java
>> --- a/src/share/classes/java/util/logging/LogRecord.java        Wed Feb 25
>> 14:32:01 2009 +0000
>> +++ b/src/share/classes/java/util/logging/LogRecord.java        Thu Mar 12
>> 17:12:22 2009 -0500
>> @@ -25,6 +25,8 @@
>>
>>  package java.util.logging;
>>  import java.util.*;
>> +import java.util.concurrent.atomic.AtomicLong;
>> +import java.util.concurrent.atomic.AtomicInteger;
>>  import java.io.*;
>>
>>  /**
>> @@ -64,9 +66,9 @@
>>  */
>>
>>  public class LogRecord implements java.io.Serializable {
>> -    private static long globalSequenceNumber;
>> -    private static int nextThreadId=10;
>> -    private static ThreadLocal<Integer> threadIds = new
>> ThreadLocal<Integer>();
>> +    private static final AtomicLong globalSequenceNumber = new
>> AtomicLong();
>> +    private static final AtomicInteger nextThreadId = new
>> AtomicInteger(10);
>> +    private static final ThreadLocal<Integer> threadIds = new
>> ThreadLocal<Integer>();
>>
>>     /**
>>      * @serial Logging message level
>> @@ -144,15 +146,13 @@
>>         this.level = level;
>>         message = msg;
>>         // Assign a thread ID and a unique sequence number.
>> -        synchronized (LogRecord.class) {
>> -            sequenceNumber = globalSequenceNumber++;
>> -            Integer id = threadIds.get();
>> -            if (id == null) {
>> -                id = new Integer(nextThreadId++);
>> -                threadIds.set(id);
>> -            }
>> -            threadID = id.intValue();
>> +        sequenceNumber = globalSequenceNumber.getAndIncrement();
>> +        Integer id = threadIds.get();
>> +        if (id == null) {
>> +            id = Integer.valueOf(nextThreadId.getAndIncrement());
>> +            threadIds.set(id);
>>         }
>> +        threadID = id.intValue();
>>         millis = System.currentTimeMillis();
>>         needToInferCaller = true;
>>    }
>>
>>