Code (Pre-)Review for JEP 280: Indify String Concat

Aleksey Shipilev aleksey.shipilev at oracle.com
Fri Nov 27 11:49:07 UTC 2015


Hi Ivan,

Thanks for the reviews. Again, I will publish the updated webrevs after
rewiring for Maurizio's comments.


On 11/27/2015 02:33 PM, Ivan Gerasimov wrote:
> 1) As you touch Long.java anyway, could you change
> 508         while (i <= Integer.MIN_VALUE) {
> to
> 508         while (i < Integer.MIN_VALUE) {
> , which may save us a half of nanosecond on some inputs?
> 
> And the same on the line# 563.

No, I think that deserves a separate round of Long/Integer.getChars
cleanups, if any. Let's not clobber the Indy String Concat with a
general optimization in the class library.


> 2) Integer.java and Long.java
> In the javadoc, just for a sake of accuracy:
> 550      * @return index of the most significant digit *or minus sign,
> if present*

Yes, this is good. Fixed.


> 3) StringConcatFactory.java
> 255                         acc = new StringBuilder();
> wouldn't it be a bit more efficient to do `acc.setLength(0)`?

Yes, this is good too. Fixed.


> 4) StringConcatFactory.java
> 
>  586             try {
>  587                 mh = generate(caller, mt, rec);
>  588             } catch (Throwable t) {
>  589                 if (t instanceof StringConcatException) {
>  590                     throw (StringConcatException)t;
>  591                 } else {
>  592                     throw new StringConcatException("Generator
> failed", t);
>  593                 }
>  594             }
> 
> it seems to be a bit more straight-forward:
>              try {
>                  mh = generate(caller, mt, rec);
>              } catch (StringConcatException sce) {
>                  throw sce;
>              } catch (Throwable t) {
>                  throw new StringConcatException("Generator failed", t);
>              }

Ah, right, this is better, fixed.

Thanks,
-Aleksey


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20151127/c735e06d/signature.asc>


More information about the compiler-dev mailing list