I like the optimizations in this change, especially the avoidance of
copies. Was there some good reason the jdk never before used JavaLangAccess
to avoid String creation copies?
I am tempted to micro-optimize this some more, e.g. specialize the hex
printing code. I might get rid of the digits table and compute ASCII
characters simply:
((i < 10) ? '0' : ('a' - 10)) + i
Why not formatUnsignedInt?
Why not make the new format methods public?
Is there a better name than createStringSharedChars? We hope those chars
will *not* be shared! How about newStringUnsafe(char[] chars)
The spec for
+ int formatUnsignedLong(long val, int shift, char[] buf, int offset,
int len);
should make it clearer whose responsibility getting the size right is. It
looks like the caller has to ensure there will be enough space, so perhaps
the caller should just pass one offset instead of offset plus len.
+ * @return the lowest character location used
stray space
On Wed, 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/
>
> 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
> >
>
>
