RFR: 8261731: shallow copy the internal buffer of a scalar-replaced java.lang.String object

[Vladimir Kozlov]

On Fri, 19 Mar 2021 06:23:10 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> Sure, I'll re-run testing.
>> 
>> After looking through the changes, I'm concerned about all the complexity that is added to handle this special case. Do you have any performance numbers other than microbenchmark results that would justify this?
>
>> Sure, I'll re-run testing.
>> 
>> After looking through the changes, I'm concerned about all the complexity that is added to handle this special case. Do you have any performance numbers other than microbenchmark results that would justify this?
> 
> hi, @TobiHartmann , 
> It's not my intention to make a big patch.  I would like to get help from reviewers. Am I making thing over complex? Is there an alternative to make thing simpler? 
> 
> The reason that the patch is big because I meet some constraints in current optimizing compiler. 
> 1. It seems all oops must be allocated on heap. There's no pseudo oop. or I can’t find a way to create a temp of fake oop.  
> 2. Scalar_replacement and ArrayCopy idealization don't support variable-length array(VLA).  String::value is a VLA. 
> 3. Lack supports of frozen array in C2's typing system. JDK-8261007
> 4. re-allocation doesn't support nested objects. j.l.String has 4 fields. String::value is an array of byte, which I need to reallocate too.
> 
> To overcome the constraints, I make the following new features in this patch.
> 1. `process_users_of_string_allocation` intercepts all uses of AllocateArray. 
> 2. reorder the trinity Allocate-AllocateArray-ArrayCopy(or alloc-aa-ac) and eliminate them in MacroExpansion phase.
> 3. `ArrayCopyNode::is_string_copy` conservatively recognizes string initialization from another string. 
> 4. invent a scheme to make deoptimization support a nested object, so deopt can reallocate an entire j.l.String object.
> 
> TBH, I haven't seen obvious performance improvement in SpecJBB. Only some microbenches can prove my idea.  When I profile a string intensive application, I found that the major blocker is JDK-8261531.  Current implementation requires the String object must be scalar-replaceable first.  A String object is NOT scalar replaceable until its length is a compiler-time constant.  It's pretty rare in the real world. Therefore, I would like to put the evaluation of performance impact on hold before I resolve JDK-8261531.
> 
> Could you help me verify that the current code can handle all code shape first?  Further, I really appreciate that you can give me some advice in terms of high-level design.
> 
> thanks,
> --lx

@navyxliu I think you want to fix a lot of issues once. You need to split problems you see.

I did few experiments and the one issue which could be fixed first is scalarizing of array created by `Arrays.copyOfRange()` used by `String::substring()`.

I see that current JDK easy scalarize new array and arraycopy():
     private static int test4(byte[] src) {
         byte[] dst = new byte[6];
         System.arraycopy(src, 1, dst, 0, 6);
         return dst[2] + dst[5];
     }
But it did not scalarize:
     private static int test5(byte[] src) {
         byte[] dst = Arrays.copyOfRange(src, 1, 7);
         return dst[2] + dst[5];
     }
Even so code in `Arrays.copyOfRange()` is the same as in `test4()`:
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/Arrays.java#L3817

Arrays does not escaped but it is not eliminated because Loads are not redirected into `dst` array as in `test4()` case.

I think this should be fixed first.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2570