tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <ch...@christopherschultz.net>
Subject Re: svn commit: r1627000 - /tomcat/trunk/java/org/apache/tomcat/util/buf/HexUtils.java
Date Tue, 23 Sep 2014 16:16:06 GMT
Konstantin,

On 9/23/14 10:10 AM, Konstantin Kolinko wrote:
> 2014-09-23 17:09 GMT+04:00  <schultz@apache.org>:
>> Author: schultz
>> Date: Tue Sep 23 13:09:42 2014
>> New Revision: 1627000
>>
>> URL: http://svn.apache.org/r1627000
>> Log:
>> Micro optimization.
>>
>> Modified:
>>     tomcat/trunk/java/org/apache/tomcat/util/buf/HexUtils.java
>>
>> Modified: tomcat/trunk/java/org/apache/tomcat/util/buf/HexUtils.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/buf/HexUtils.java?rev=1627000&r1=1626999&r2=1627000&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/tomcat/util/buf/HexUtils.java (original)
>> +++ tomcat/trunk/java/org/apache/tomcat/util/buf/HexUtils.java Tue Sep 23 13:09:42
2014
>> @@ -75,10 +75,12 @@ public final class HexUtils {
>>          if (null == bytes) {
>>              return null;
>>          }
>> +        final char[] hex = HexUtils.hex;
>> +        final int length = bytes.length;
> 
> The above change does not make sense to me.
> 
> As HexUtils.hex and bytes.length are themselves final fields, it is up
> to Java JVM to inline them. I see no need to define explicit local
> fields for them.

Seems there are major objections to small performance improvements. I'll
revert the change.

>> -        StringBuilder sb = new StringBuilder(bytes.length << 1);
>> +        StringBuilder sb = new StringBuilder(length << 1);
>>
>> -        for(int i = 0; i < bytes.length; ++i) {
>> +        for(int i = 0; i < length; ++i) {
>>              sb.append(hex[(bytes[i] & 0xf0) >> 4])
>>                  .append(hex[(bytes[i] & 0x0f)])
>>                  ;
>> @@ -94,8 +96,9 @@ public final class HexUtils {
>>          }
>>
>>          char[] inputChars = input.toCharArray();
>> -        byte[] result = new byte[input.length() >> 1];
>> -        for (int i = 0; i < result.length; i++) {
>> +        final int length = input.length() >> 1;
>> +        byte[] result = new byte[length];
>> +        for (int i = 0; i < length; i++) {
> 
> This fromHexString() method (where the above lines are) is used by
> test code only.
> 
> The usefulness of this fromHexString() method is somewhat limited, as
> it does not check correctness of its arguments. If input string has
> even number of chars, the last char will be silently ignored. If some
> characters are non-hex, it will silently produce bogus results.

This method will become more useful when bug
https://issues.apache.org/bugzilla/show_bug.cgi?id=56403 is implemented.
Thanks for the review of the code; we'll make appropriate modifications.

-chris


Mime
View raw message