<AWT Dev> RFR: 8264428: Replace uses of StringBuffer with StringBuilder in java.desktop

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

<AWT Dev> RFR: 8264428: Replace uses of StringBuffer with StringBuilder in java.desktop

Andrey Turbanov-2
Found by IntelliJ IDEA inspection `Java | Java language level migration aids | Java 5 | 'StringBuffer' may be 'StringBuilder'`
As suggested in https://github.com/openjdk/jdk/pull/1507#issuecomment-757369003 I've created separate PR for module `java.desktop`
Similar cleanups in the past: https://bugs.openjdk.java.net/browse/JDK-8041679 https://bugs.openjdk.java.net/browse/JDK-8264029

-------------

Commit messages:
 - [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop

Changes: https://git.openjdk.java.net/jdk/pull/3251/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3251&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264428
  Stats: 44 lines in 11 files changed: 0 ins; 8 del; 36 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3251.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3251/head:pull/3251

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Replace uses of StringBuffer with StringBuilder in java.desktop

Aleksey Shipilev-5
On Mon, 29 Mar 2021 20:50:14 GMT, Andrey Turbanov <[hidden email]> wrote:

> Found by IntelliJ IDEA inspection `Java | Java language level migration aids | Java 5 | 'StringBuffer' may be 'StringBuilder'`
> As suggested in https://github.com/openjdk/jdk/pull/1507#issuecomment-757369003 I've created separate PR for module `java.desktop`
> Similar cleanups in the past: https://bugs.openjdk.java.net/browse/JDK-8041679 https://bugs.openjdk.java.net/browse/JDK-8264029

Submitted: https://bugs.openjdk.java.net/browse/JDK-8264428, please rename PR to "8264428: Replace uses of StringBuffer with StringBuilder in java.desktop".

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Replace uses of StringBuffer with StringBuilder in java.desktop

Andrey Turbanov-2
In reply to this post by Andrey Turbanov-2
On Mon, 29 Mar 2021 20:50:14 GMT, Andrey Turbanov <[hidden email]> wrote:

> Found by IntelliJ IDEA inspection `Java | Java language level migration aids | Java 5 | 'StringBuffer' may be 'StringBuilder'`
> As suggested in https://github.com/openjdk/jdk/pull/1507#issuecomment-757369003 I've created separate PR for module `java.desktop`
> Similar cleanups in the past: https://bugs.openjdk.java.net/browse/JDK-8041679 https://bugs.openjdk.java.net/browse/JDK-8264029

src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFFieldNode.java line 127:

> 125:                 StringBuilder sb = new StringBuilder();
> 126:                 for (int i = 0; i < count; i++) {
> 127:                     sb.append((data[i] & 0xff));

Remove redundant parentheses

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Replace uses of StringBuffer with StringBuilder in java.desktop [v2]

Andrey Turbanov-2
In reply to this post by Andrey Turbanov-2
> Found by IntelliJ IDEA inspection `Java | Java language level migration aids | Java 5 | 'StringBuffer' may be 'StringBuilder'`
> As suggested in https://github.com/openjdk/jdk/pull/1507#issuecomment-757369003 I've created separate PR for module `java.desktop`
> Similar cleanups in the past: https://bugs.openjdk.java.net/browse/JDK-8041679 https://bugs.openjdk.java.net/browse/JDK-8264029

Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:

  [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3251/files
  - new: https://git.openjdk.java.net/jdk/pull/3251/files/9bd4e523..80bff1db

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3251&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3251&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3251.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3251/head:pull/3251

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Replace uses of StringBuffer with StringBuilder in java.desktop [v2]

Andrey Turbanov-2
In reply to this post by Andrey Turbanov-2
On Tue, 30 Mar 2021 18:31:34 GMT, Andrey Turbanov <[hidden email]> wrote:

>> Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop
>
> src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFFieldNode.java line 127:
>
>> 125:                 StringBuilder sb = new StringBuilder();
>> 126:                 for (int i = 0; i < count; i++) {
>> 127:                     sb.append((data[i] & 0xff));
>
> Remove redundant parentheses

fixed.
It's suspicious that IDEA wasn't able to remove them by itself. Reported https://youtrack.jetbrains.com/issue/IDEA-265548

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Replace uses of StringBuffer with StringBuilder in java.desktop

Sergey Bylokhov-2
In reply to this post by Aleksey Shipilev-5
On Tue, 30 Mar 2021 12:39:15 GMT, Aleksey Shipilev <[hidden email]> wrote:

>> Found by IntelliJ IDEA inspection `Java | Java language level migration aids | Java 5 | 'StringBuffer' may be 'StringBuilder'`
>> As suggested in https://github.com/openjdk/jdk/pull/1507#issuecomment-757369003 I've created separate PR for module `java.desktop`
>> Similar cleanups in the past: https://bugs.openjdk.java.net/browse/JDK-8041679 https://bugs.openjdk.java.net/browse/JDK-8264029
>
> Submitted: https://bugs.openjdk.java.net/browse/JDK-8264428, please rename PR to "8264428: Replace uses of StringBuffer with StringBuilder in java.desktop".

Looks like this change goes beyond of simple replacement of  StringBuffer with StringBuilder. Please update the description of the bug and PR description.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Cleanup usages of StringBuffer in java.desktop

Andrey Turbanov-2
On Wed, 31 Mar 2021 21:12:37 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> Submitted: https://bugs.openjdk.java.net/browse/JDK-8264428, please rename PR to "8264428: Replace uses of StringBuffer with StringBuilder in java.desktop".
>
> Looks like this change goes beyond of simple replacement of  StringBuffer with StringBuilder. Please update the description of the bug and PR description.

I've updated PR description, but I don't have rights to update JIRA.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Cleanup usages of StringBuffer in java.desktop

Aleksey Shipilev-5
On Thu, 1 Apr 2021 12:48:52 GMT, Andrey Turbanov <[hidden email]> wrote:

>> Looks like this change goes beyond of simple replacement of  StringBuffer with StringBuilder. Please update the description of the bug and PR description.
>
> I've updated PR description, but I don't have rights to update JIRA.

I updated the bug synopsis in JIRA.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v2]

Alexander Zvegintsev-2
In reply to this post by Andrey Turbanov-2
On Tue, 30 Mar 2021 19:05:37 GMT, Andrey Turbanov <[hidden email]> wrote:

>> There are few possible cleanups in java.desktop related to legacy StringBuffer usages:
>> 1. In few places StringBuffer can be replaced with plain String concatenation.
>> 2. StringBuffer can be replaced with StringBuilder. StringBuilder has better performance as it is not thread-safe.
>> 3. There are few places where result of string concatenation is passed to StringBuffer.append method. Using separate `.append` calls is more clear.
>> 4. In few places primitives are unnecessary converted to String before `.append` call. They can be replaced with specialized methods (like `.append(int)` calls.
>
> Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:
>
>   [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop

src/java.desktop/windows/classes/sun/java2d/d3d/D3DContext.java line 139:

> 137:         @Override
> 138:         public String toString() {
> 139:             StringBuilder buf = new StringBuilder(super.toString());

Looks like this is the only file where copyright year is not updated.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v3]

Andrey Turbanov-2
In reply to this post by Andrey Turbanov-2
> There are few possible cleanups in java.desktop related to legacy StringBuffer usages:
> 1. In few places StringBuffer can be replaced with plain String concatenation.
> 2. StringBuffer can be replaced with StringBuilder. StringBuilder has better performance as it is not thread-safe.
> 3. There are few places where result of string concatenation is passed to StringBuffer.append method. Using separate `.append` calls is more clear.
> 4. In few places primitives are unnecessary converted to String before `.append` call. They can be replaced with specialized methods (like `.append(int)` calls.

Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:

  [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop
 
  fix copyright year

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3251/files
  - new: https://git.openjdk.java.net/jdk/pull/3251/files/80bff1db..963dc56e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3251&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3251&range=01-02

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3251.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3251/head:pull/3251

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v3]

Andrey Turbanov-2
In reply to this post by Alexander Zvegintsev-2
On Tue, 30 Mar 2021 15:46:56 GMT, Alexander Zvegintsev <[hidden email]> wrote:

>> Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop
>>  
>>   fix copyright year
>
> src/java.desktop/windows/classes/sun/java2d/d3d/D3DContext.java line 139:
>
>> 137:         @Override
>> 138:         public String toString() {
>> 139:             StringBuilder buf = new StringBuilder(super.toString());
>
> Looks like this is the only file where copyright year is not updated.

Thanks. Fixed.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v3]

Alexander Zvegintsev-2
In reply to this post by Andrey Turbanov-2
On Wed, 7 Apr 2021 06:39:48 GMT, Andrey Turbanov <[hidden email]> wrote:

>> There are few possible cleanups in java.desktop related to legacy StringBuffer usages:
>> 1. In few places StringBuffer can be replaced with plain String concatenation.
>> 2. StringBuffer can be replaced with StringBuilder. StringBuilder has better performance as it is not thread-safe.
>> 3. There are few places where result of string concatenation is passed to StringBuffer.append method. Using separate `.append` calls is more clear.
>> 4. In few places primitives are unnecessary converted to String before `.append` call. They can be replaced with specialized methods (like `.append(int)` calls.
>
> Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:
>
>   [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop
>  
>   fix copyright year

Marked as reviewed by azvegint (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v3]

Alexey Ivanov-2
In reply to this post by Andrey Turbanov-2
On Wed, 7 Apr 2021 06:39:48 GMT, Andrey Turbanov <[hidden email]> wrote:

>> There are few possible cleanups in java.desktop related to legacy StringBuffer usages:
>> 1. In few places StringBuffer can be replaced with plain String concatenation.
>> 2. StringBuffer can be replaced with StringBuilder. StringBuilder has better performance as it is not thread-safe.
>> 3. There are few places where result of string concatenation is passed to StringBuffer.append method. Using separate `.append` calls is more clear.
>> 4. In few places primitives are unnecessary converted to String before `.append` call. They can be replaced with specialized methods (like `.append(int)` calls.
>
> Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:
>
>   [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop
>  
>   fix copyright year

Marked as reviewed by aivanov (Reviewer).

src/java.desktop/unix/classes/sun/awt/X11/XCreateWindowParams.java line 88:

> 86:         while (eIter.hasNext()) {
> 87:             Map.Entry<Object, Object> entry = eIter.next();
> 88:             buf.append(entry.getKey()).append(": ").append(entry.getValue()).append("\n");

Would it be clearer if each append was on its own line?
Suggestion:

            buf.append(entry.getKey())
               .append(": ")
               .append(entry.getValue())
               .append("\n");

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v4]

Andrey Turbanov-2
In reply to this post by Andrey Turbanov-2
> There are few possible cleanups in java.desktop related to legacy StringBuffer usages:
> 1. In few places StringBuffer can be replaced with plain String concatenation.
> 2. StringBuffer can be replaced with StringBuilder. StringBuilder has better performance as it is not thread-safe.
> 3. There are few places where result of string concatenation is passed to StringBuffer.append method. Using separate `.append` calls is more clear.
> 4. In few places primitives are unnecessary converted to String before `.append` call. They can be replaced with specialized methods (like `.append(int)` calls.

Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:

  8264484: Replace uses of StringBuffer with StringBuilder in jdk.hotspot.agent
 
  place each 'append' call on its own line

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3251/files
  - new: https://git.openjdk.java.net/jdk/pull/3251/files/963dc56e..d083f953

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3251&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3251&range=02-03

  Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3251.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3251/head:pull/3251

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v3]

Andrey Turbanov-2
In reply to this post by Alexey Ivanov-2
On Wed, 7 Apr 2021 15:21:40 GMT, Alexey Ivanov <[hidden email]> wrote:

>> Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop
>>  
>>   fix copyright year
>
> src/java.desktop/unix/classes/sun/awt/X11/XCreateWindowParams.java line 88:
>
>> 86:         while (eIter.hasNext()) {
>> 87:             Map.Entry<Object, Object> entry = eIter.next();
>> 88:             buf.append(entry.getKey()).append(": ").append(entry.getValue()).append("\n");
>
> Would it be clearer if each append was on its own line?
> Suggestion:
>
>             buf.append(entry.getKey())
>                .append(": ")
>                .append(entry.getValue())
>                .append("\n");

done

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v4]

Alexey Ivanov-2
In reply to this post by Andrey Turbanov-2
On Thu, 8 Apr 2021 14:43:50 GMT, Andrey Turbanov <[hidden email]> wrote:

>> There are few possible cleanups in java.desktop related to legacy StringBuffer usages:
>> 1. In few places StringBuffer can be replaced with plain String concatenation.
>> 2. StringBuffer can be replaced with StringBuilder. StringBuilder has better performance as it is not thread-safe.
>> 3. There are few places where result of string concatenation is passed to StringBuffer.append method. Using separate `.append` calls is more clear.
>> 4. In few places primitives are unnecessary converted to String before `.append` call. They can be replaced with specialized methods (like `.append(int)` calls.
>
> Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:
>
>   8264484: Replace uses of StringBuffer with StringBuilder in jdk.hotspot.agent
>  
>   place each 'append' call on its own line

Marked as reviewed by aivanov (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v4]

Alexander Zvegintsev-2
In reply to this post by Andrey Turbanov-2
On Thu, 8 Apr 2021 14:43:50 GMT, Andrey Turbanov <[hidden email]> wrote:

>> There are few possible cleanups in java.desktop related to legacy StringBuffer usages:
>> 1. In few places StringBuffer can be replaced with plain String concatenation.
>> 2. StringBuffer can be replaced with StringBuilder. StringBuilder has better performance as it is not thread-safe.
>> 3. There are few places where result of string concatenation is passed to StringBuffer.append method. Using separate `.append` calls is more clear.
>> 4. In few places primitives are unnecessary converted to String before `.append` call. They can be replaced with specialized methods (like `.append(int)` calls.
>
> Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:
>
>   8264484: Replace uses of StringBuffer with StringBuilder in jdk.hotspot.agent
>  
>   place each 'append' call on its own line

Marked as reviewed by azvegint (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251
Reply | Threaded
Open this post in threaded view
|

<AWT Dev> Integrated: 8264428: Cleanup usages of StringBuffer in java.desktop

Andrey Turbanov-2
In reply to this post by Andrey Turbanov-2
On Mon, 29 Mar 2021 20:50:14 GMT, Andrey Turbanov <[hidden email]> wrote:

> There are few possible cleanups in java.desktop related to legacy StringBuffer usages:
> 1. In few places StringBuffer can be replaced with plain String concatenation.
> 2. StringBuffer can be replaced with StringBuilder. StringBuilder has better performance as it is not thread-safe.
> 3. There are few places where result of string concatenation is passed to StringBuffer.append method. Using separate `.append` calls is more clear.
> 4. In few places primitives are unnecessary converted to String before `.append` call. They can be replaced with specialized methods (like `.append(int)` calls.

This pull request has now been integrated.

Changeset: 8a235807
Author:    Andrey Turbanov <[hidden email]>
Committer: Alexander Zvegintsev <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/8a235807
Stats:     48 lines in 11 files changed: 3 ins; 8 del; 37 mod

8264428: Cleanup usages of StringBuffer in java.desktop

Reviewed-by: azvegint, aivanov

-------------

PR: https://git.openjdk.java.net/jdk/pull/3251