On Feb 13, 2013, at 2:45 PM, Mike Duigou <mike.dui...@oracle.com> wrote:
> I have updated the patch with some of Ulf's feedback and corrected one > cut-and-paste error that I made. > > The updated webrev is at: > > http://cr.openjdk.java.net/~mduigou/JDK-8006627/2/webrev/ > One last ping to make sure this doesn't slip through the cracks, as I haven't seen the commit go through the public JDK8 repo… or maybe I missed it. Thanks again for looking at this :-) > Mike > > On Feb 12 2013, at 18:25 , Ulf Zibis wrote: > >> Am 13.02.2013 02:34, schrieb Mike Duigou: >>> Thank you for the comments Ulf. >>> >>> On Feb 12 2013, at 17:24 , Ulf Zibis wrote: >>> >>>> Am 13.02.2013 00:30, schrieb Mike Duigou: >>>>> Hi Steven; >>>>> >>>>> I have updated the patch for Java 8. There's somewhat less code sharing >>>>> and a bit of refactoring than your last version but the performance >>>>> should be about the same or a little better. >>>>> >>>>> http://cr.openjdk.java.net/~mduigou/JDK-8007398/0/webrev/ >>>> Couldn't you use String(buf, true) for all to(Unsigned)String(...) methods >>>> ? >>> Possibly. I didn't go looking too far with looking for additional >>> improvements. >>> >>>> Instead of calculating the mask each time, you could use: >>>> 309 private static String toUnsignedString(int i, int shift, int mask) >>>> { >>> I think that would actually be inefficient. I haven't looked at the JITed >>> code but the mask calculation is pretty cheap relative to parameter passing. >> >> I believe, JIT will inline the code, so there would be no parameter passing. >> >> Additionally the calculation of char count could be faster, if you would >> have short cuts like: >> numberOfLeading2Zeros(i) >> numberOfLeading4Zeros(i) >> numberOfLeading8Zeros(i) >> ... >> So the optimum would be with separate toString methods: >> String toBase2String(int i); >> String toBase4String(int i); >> String toBase8String(int i); >> ... >> >> In any case I would save 2 lines: >> >> 311 int mag = Integer.SIZE - Long.numberOfLeadingZeros(i); >> 312 int charCount = Math.max(((mag + (shift - 1)) / shift), 1); >> 313 char[] buf = new char[charCount]; >> 316 int mask = (1 << shift) - 1; >> >> -Ulf >> >
