Bug #72008 Useless object creation in StringUtils#getBytes-methods

• Submitted: 11 Mar 2014 11:25
• Modified: 3 Apr 2014 23:23
• Reporter: Andrej Golovnin (OCA)
• Status: Closed
• Category: Connector / J
• Severity: S5 (Performance)
• Version: 5.1.29
• OS: Any
• Assigned to: Filipe Silva
• CPU Architecture: Any
• Tags: jdbc, StringUtils

[11 Mar 2014 11:25] Andrej Golovnin

Description:
StringUtils#getBytes-methods, which take a character array as argument, create a temporary String object to encode the character array into bytes. But the constructor String(char[]) always creates a copy of the original character array. To avoid array copying the combination of CharBuffer and Charset should be used instead.

Some of StringUtils#getBytes-methods make useless copy of the byte array. See attached patch for details.

How to repeat:
Review the code of the StringUtils#getBytes-methods.

Suggested fix:
See attached patch. The patch also removes "final" keyword from some static methods as marking static-methods as final doesn't have any benefit. The patch also changes the method #fixDecimalExponent to use character-based String#indexOf method as it is faster then the String-based method.

[11 Mar 2014 11:26] Andrej Golovnin

Patch for this issue.

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: StringUtils.patch (application/octet-stream, text), 7.55 KiB.

[12 Mar 2014 14:01] Alexander Soklakov

Hi Andrej,

Thank you again!
Verified by code review.

[3 Apr 2014 23:23] Daniel So

Added the following entry into the Connector/J 5.1.31 changelog:

"The StringUtils.getBytes methods have been refactored to avoid unnecessary creations of string objects for encoding character arrays into bytes. Also, unneeded StringBuffers are replaced by StringBuiders."

[23 Jun 2014 10:28] Ville Skyttä

Marking static methods as final *does* have a benefit: final static methods cannot be overridden/hidden in subclasses while non-final static ones can. I suggest reverting this part of the change.

http://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.4.3.3

[2 Jul 2014 0:11] Johnathan Crawford

I believe this enhancement has caused a regression with certain prepared statement queries, please see http://bugs.mysql.com/bug.php?id=73163

[4 Aug 2014 10:31] Filipe Silva

Ville Skyttä, you're right. But this class isn't meant to be instantiated neither subclassed. Its methods are called internally or externally by fully qualified name, so there is no risk of overriding/hiding them.

In terms of code design specifications, we decided not to mark any method or class as "final" unless we explicitly want to state that overriding or subclassing them have serious design implications.

Thanks,