Hi Brian,
Looks good to me.
On 02/10/2019 01:13, Brian Burkhalter wrote:
While the performance improvement that I measured for this proposed change [1]
using JMH was only in the 2% to 8% range as opposed to the 13% claimed in the
issue description, given the simplicity of the change [2] it is probably worth
doing. All use of the StringBuilder is within a synchronized block within a
single package-scope method, so there should be no problem with access by
multiple threads.
The StringBuffer is a local in the readLine method and
doesn't escape - so whether or not it's accessed within a synchronized
block should not matter. It's safe to replace it with StringBuilder
indeed!
best regards,
-- daniel
Thanks,
Brian
[1] https://bugs.openjdk.java.net/browse/JDK-8229022
[2] diff
--- a/src/java.base/share/classes/java/io/BufferedReader.java
+++ b/src/java.base/share/classes/java/io/BufferedReader.java
@@ -314,7 +314,7 @@
* @throws IOException If an I/O error occurs
*/
String readLine(boolean ignoreLF, boolean[] term) throws IOException {
- StringBuffer s = null;
+ StringBuilder s = null;
int startChar;
synchronized (lock) {
@@ -372,7 +372,7 @@
}
if (s == null)
- s = new StringBuffer(defaultExpectedLineLength);
+ s = new StringBuilder(defaultExpectedLineLength);
s.append(cb, startChar, i - startChar);
}
}