RFR 8245302: Upgrade LogRecord to support long thread ids and remove its usage of ThreadLocal

[Alan Bateman]

On 10/06/2020 16:06, Rahul wrote:
>    Hello,
>
>   
>
>    Request to have my fix reviewed for the issue:
>
>    
>
>    JDK-8245302:  Upgrade LogRecord to support long thread ids and remove its usage of ThreadLocal.
>
>     
>
>    java.util.logging.LogRecord is updated to support long thread ids without ThreadLocal.
>
>    Before the update, thread id’s did not support long values, ThreadLocal usage has been
>
>    Removed.
>
> .
>
>   
>
>    issue:       https://bugs.openjdk.java.net/browse/JDK-8245302
>
>    webrev:   http://cr.openjdk.java.net/~ryadav/webrev_8245302/index.html
>
>    csr:           https://bugs.openjdk.java.net/browse/JDK-8247219
>
>
The addition of setLongThreadID/getLongThreadID, and deprecating the old 
methods looks good. One suggestion is that setLongThreadID can return 
the LogRecord (= this) to allow for method invocation chaining. For the 
javadoc then it would be clearer to say "Returns the ..." rather than 
"Get a ..." because a sequence of calls to getXXX has to return the same 
thread ID.

LogRecord is serializable but it looks like like you've thought about 
all the issue so I think this looks.

shortThreadID looks a bit strange in that I would have expected it would 
just return tid when in the 0-Integer.MAX_VALUE range and compute a 
negative value when it's larger than Integer.MAX_VALUE. It's true that 
the old behavior was to switch to dense values when larger than 
MAX_VALUE/2 but I can't imagine how anything could depend on this. So 
not objecting to the approach, just pointing out that there may be 
simpler alternatives that could be explored (if you haven't done so 
already).

The overall test cover looks good. Can you provide clear instructions in 
SerializeLogRecordTest on how to re-create the base64 string of the 
serialized form? That is important for future maintainers.

There are several formatting/style nits in the patch but they aren't 
important for now.

-Alan.