On Apr 30 2013, at 23:41 , Peter Levart wrote:
> Hi Mike,
>
> Some comments about the test...
>
> 40 String benchmark = "exemplar";
> 41 String constructorCopy = new String(benchmark);
> 42 String jlaCopy = jla.newStringUnsafe(benchmark.toCharArray());
> 43
> 44 if (constructorCopy == constructorCopy) {
> 45 throw new Error("should be different instances");
> 46 }
> Wouldn't this always throw error? You really wanted to test benchmark ==
> constructorCopy, right?
Argh, yes. I forgot a hg qrefresh before generating the webrev. This was
corrected in my local copy.
> To check whether the jlaCopy is really taking the given char array by
> reference, a test could also do something "illegal" like:
>
>
> char[] array = benchmark.toCharArray();
> String jlaCopy = jla.newStringUnsafe(array);
> array[0] = "X"; // ouch!
> String constructorCopy = new String(array);
>
> if (!constructorCopy.equals(jlaCopy)) { ... }
>
> Regards, Peter
>
I have added an "evil" test to check exactly that.
I'm pleased to be able to include you an official reviewer. :-)
Thanks,
Mike
>
>
>
> 2013/5/1 Mike Duigou <mike.dui...@oracle.com>
> Hello all;
>
> Since this code will be introduced without any usages I decided it was
> critical to make a stand alone unit test. I've updated the webrev:
>
> http://cr.openjdk.java.net/~mduigou/JDK-8013528/1/webrev/
>
> The webrev mistakes my hg copy for a rename... Ignore that. Capturing the
> provenance of the test file probably isn't critical since the file is
> repurposed for a different test, but I prefer to have the origin tracked
> rather than use a non-vcs copy.
>
> I also made the other changes suggested by reviewers.
>
> Thanks
>
> Mike
>
> On Apr 29 2013, at 21:30 , Mike Duigou wrote:
>
> > Hello all;
> >
> > This change originated as part of JDK-8006627 (which was also previously
> > split into JDK-8007398 as well). It adds an internal mechanism for
> > performance sensitive usages to create a string from a provided character
> > array without copying that array. This saves both in the allocation (and
> > subsequent GC) as well as the copying of the characters. There are a few
> > places in the JDK that return Strings which can benefit from this change.
> >
> > http://cr.openjdk.java.net/~mduigou/JDK-8013528/0/webrev/
> >
> > Fear not, JDK-8006627 and JDK-8007398 will be revisited... For now it would
> > be to get this change in to allow other potential users to move forward
> > with their changes.
> >
> > Mike
>
>
