On 23/09/2014 17:16, Christopher Schultz wrote:
> Konstantin,
>
> On 9/23/14 10:10 AM, Konstantin Kolinko wrote:
>> 2014-09-23 17:09 GMT+04:00 <schu...@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.
I disagree with that characterisation. The objection was that it added
more code for no demonstrable gain. If a micro-benchmark can demonstrate
some quantifiable improvement I'd be happy to take another look.
>>> - 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.
Agreed.
Mark
> Thanks for the review of the code; we'll make appropriate modifications.
>
> -chris
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
