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/ 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 >
