On Jan 30, 2013, at 11:42 AM, Mike Duigou <mike.dui...@oracle.com> wrote:
> Good work Steven! Thanks for the review :) > > Some initial comments; > > - The changes to Long should be in a separate issue. It's generally > encouraged for changesets to focus on only one change. Sorry, yes, it's > additional overhead. That's unfortunate. I don't have the ability / know how to file a second, separate issue, and I've already spent a lot more time trying to get this small change through than I'd expected on overhead. Will it be preferable to have someone open a second issue, make two web revs, and code review both, or should I just abandon the attempt at code-sharing between UUID and Long? In the original patch that Aleksey reviewed I had not attempted this; I did it on his suggestion. > > - I would like to see if performed of toString() can be improved further by > using String(char[] value, boolean share) constructor via a > sun.miscSharedSecret.JavaLangAccesss method to construct the string directly > from the character array. You could test to see if this has positive benefit > by temporarily using a static char array. I will incorporate this into my next revision > > - public static String toString(long msb, long lsb) should be private. > There's no compelling reason to add this to the API. > I can remove this. I added it on the theory that if you wish to ship UUIDs around as two longs, it may be useful to omit UUID object instantiation just to format as a string. Maybe this is too far out there to make another public method acceptable. > - Have you run this code against any of the existing regression tests? Yes, I ran the jtreg UUID and Long tests, all pass. I ran the Apache Harmony UUID test cases against the pre-integrated version of the code. (There should only have been minor modifications since then, variable renamings, whatnot…) > > Mike > > On Jan 28 2013, at 19:23 , Steven Schlansker wrote: > >> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8006627 >> >> I have created a patch that dramatically improves UUID to/from string >> performance. >> Please find below a webrev with my proposed changes. >> >> Thanks in advance for any feedback on the contents. >> I do not believe I have a committer lined up yet. >> My company has a signed OCA on file, "Ness Computing". >> >> http://dl.dropbox.com/u/1422321/uuid_webrev/index.html >> http://dl.dropbox.com/u/1422321/uuid_webrev.zip >> >
