Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Sat, 20 Feb 2021 12:17:32 GMT, Сергей Цыпанов wrote: > This is a very simple and trivial improvement about getting rid of pointless > char wrapping into array Marked as reviewed by azvegint (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Sat, 20 Feb 2021 12:17:32 GMT, Сергей Цыпанов wrote: > This is a very simple and trivial improvement about getting rid of pointless > char wrapping into array LGTM - Marked as reviewed by vtewari (Committer). PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Sat, 13 Mar 2021 15:19:18 GMT, Claes Redestad wrote: >> This is a very simple and trivial improvement about getting rid of pointless >> char wrapping into array > > LGTM I'll sponsor when I'm back to work on Monday, assuming there are no objections. - PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Sat, 13 Mar 2021 15:18:59 GMT, Claes Redestad wrote:
>> This is a very simple and trivial improvement about getting rid of pointless
>> char wrapping into array
>
> src/java.base/share/classes/java/io/ObjectStreamClass.java line 833:
>
>> 831: String fname = in.readUTF();
>> 832: String signature = ((tcode == 'L') || (tcode == '[')) ?
>> 833: in.readTypeString() : String.valueOf(tcode);
>
> Since the result of String.valueOf here will be equal to one of the primitive
> signatures strings ("I", "J", ...) (otherwise ObjectStreamField will throw an
> exception) it might make sense to turn this into a switch expression and
> simplify the whole thing. I can't tell how performance sensitive this piece
> of code is, though.
Hi, I'll check and add one more PR if it makes sense
-
PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Sat, 20 Feb 2021 12:17:32 GMT, Сергей Цыпанов
wrote:
> This is a very simple and trivial improvement about getting rid of pointless
> char wrapping into array
LGTM
src/java.base/share/classes/java/io/ObjectStreamClass.java line 833:
> 831: String fname = in.readUTF();
> 832: String signature = ((tcode == 'L') || (tcode == '[')) ?
> 833: in.readTypeString() : String.valueOf(tcode);
Since the result of String.valueOf here will be equal to one of the primitive
signatures strings ("I", "J", ...) (otherwise ObjectStreamField will throw an
exception) it might make sense to turn this into a switch expression and
simplify the whole thing. I can't tell how performance sensitive this piece of
code is, though.
-
Marked as reviewed by redestad (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Sat, 13 Mar 2021 12:16:57 GMT, Yi Yang wrote: >> @kelthuzadx hi! I'd appreciate this, as there's no JBS issue for this ( > > Hi @stsypanov, I've created it > https://bugs.openjdk.java.net/browse/JDK-8263552. Good luck :-) Thanks! - PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Sat, 13 Mar 2021 11:35:48 GMT, Сергей Цыпанов wrote: >> Nice cleanup. I can help file a JBS issue if @c-cleary doesn't notice your >> comment. > > @kelthuzadx hi! I'd appreciate this, as there's no JBS issue for this ( Hi @stsypanov, I've created it https://bugs.openjdk.java.net/browse/JDK-8263552. Good luck :-) - PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Mon, 22 Feb 2021 12:04:14 GMT, Conor Cleary wrote: >> This is a very simple and trivial improvement about getting rid of pointless >> char wrapping into array > > src/java.base/share/classes/java/io/ObjectStreamClass.java line 833: > >> 831: String fname = in.readUTF(); >> 832: String signature = ((tcode == 'L') || (tcode == '[')) ? >> 833: in.readTypeString() : String.valueOf(tcode); > > Certainly more readable and it seems that the call to valueOf is equivalent > to whay takes place with the original code. I can't see any difference > semantically or performance-wise at a glance. LGTM Nice cleanup. I can help file a JBS issue if @c-cleary doesn't notice your comment. - PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Sat, 13 Mar 2021 03:12:32 GMT, Yi Yang wrote: >> src/java.base/share/classes/java/io/ObjectStreamClass.java line 833: >> >>> 831: String fname = in.readUTF(); >>> 832: String signature = ((tcode == 'L') || (tcode == '[')) ? >>> 833: in.readTypeString() : String.valueOf(tcode); >> >> Certainly more readable and it seems that the call to valueOf is equivalent >> to whay takes place with the original code. I can't see any difference >> semantically or performance-wise at a glance. LGTM > > Nice cleanup. I can help file a JBS issue if @c-cleary doesn't notice your > comment. @kelthuzadx hi! I'd appreciate this, as there's no JBS issue for this ( - PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Mon, 22 Feb 2021 12:04:14 GMT, Conor Cleary wrote: >> This is a very simple and trivial improvement about getting rid of pointless >> char wrapping into array > > src/java.base/share/classes/java/io/ObjectStreamClass.java line 833: > >> 831: String fname = in.readUTF(); >> 832: String signature = ((tcode == 'L') || (tcode == '[')) ? >> 833: in.readTypeString() : String.valueOf(tcode); > > Certainly more readable and it seems that the call to valueOf is equivalent > to whay takes place with the original code. I can't see any difference > semantically or performance-wise at a glance. LGTM @c-cleary Thanks for review. The difference is about intermediate array: `String.valueOf()` doesn't allocate it being slightly faster and less memory-consuming. Could you file an issue to track this? I'm not an Oracle employee and not able to do it myself. - PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Sat, 20 Feb 2021 12:17:32 GMT, Сергей Цыпанов wrote: > This is a very simple and trivial improvement about getting rid of pointless > char wrapping into array src/java.base/share/classes/java/io/ObjectStreamClass.java line 833: > 831: String fname = in.readUTF(); > 832: String signature = ((tcode == 'L') || (tcode == '[')) ? > 833: in.readTypeString() : String.valueOf(tcode); Certainly more readable and it seems that the call to valueOf is equivalent to whay takes place with the original code. I can't see any difference semantically or performance-wise at a glance. LGTM - PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Mon, 1 Mar 2021 12:50:35 GMT, Andrey Turbanov wrote: > I think it's worth to cleanup other places with similar code too. Done - PR: https://git.openjdk.java.net/jdk/pull/2660
Re: RFR: 8263552: Use String.valueOf() for char-to-String conversion in ObjectStreamClass
On Sat, 20 Feb 2021 12:17:32 GMT, Сергей Цыпанов wrote: > This is a very simple and trivial improvement about getting rid of pointless > char wrapping into array Marked as reviewed by turban...@github.com (no known OpenJDK username). I think it's worth to cleanup other places with similar code too. I found 6 places in JDK codebase:  - PR: https://git.openjdk.java.net/jdk/pull/2660
