RFR : 8013528 : (XS) Provide SharedSecrets access to String(char[], boolean) constructor

[Peter Levart]

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?
  47         if (!benchmark.equals(constructorCopy)) {  48
throw new Error("Copy not equal");  49         }  50         if (0 !=
Objects.compare(benchmark, constructorCopy,
Comparators.naturalOrder())) {  51             throw new Error("Copy
not equal");  52         }
  53   54         if (benchmark == jlaCopy) {  55             throw
new Error("should be different instances");  56         }  57
if (!benchmark.equals(jlaCopy)) {  58             throw new
Error("Copy not equal");  59         }  60         if (0 !=
Objects.compare(benchmark, jlaCopy, Comparators.naturalOrder())) {  61
            throw new Error("Copy not equal");  62         }  63   64
       if (constructorCopy == jlaCopy) {  65             throw new
Error("should be different instances");  66         }  67         if
(!constructorCopy.equals(jlaCopy)) {  68             throw new
Error("Copy not equal");  69         }  70         if (0 !=
Objects.compare(constructorCopy, jlaCopy, Comparators.naturalOrder()))
{  71             throw new Error("Copy not equal");


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





2013/5/1 Mike Duigou <mike.duigou at 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
>
>