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

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Xin Liu
> There are 3 nodes involving in the construction of a java.lang.String object.
> 1. Allocate of itself, aka. alloc
> 2. AllocateArray of a byte array, which is value:byte[], aka. aa
> 3. ArrayCopyNode which copys in the contents of value, aka. ac
>
> Lemma
> When a String object `alloc` is scalar replaced, C2 can eliminate `aa` and `ac`.
>
> Because `alloc` is scalar replaced, it must be non-escaped. The field value:byte[] of j.l.String cannot be seen by external world, therefore it must not be global escaped. Because the buffer is marked as stable, it is safe to assume its contents are whatever ac copies in. Because all public java.lang.String constructors clone the incoming array, the source of `ac` is stable as well.
>
> It is possible to rewire `aa` to the source of ac with the correct offset. That is to say, we can replace both `aa` and `ac` with a “shallow copy” of the source of `ac`. It’s safe if C2 keeps a reference of the source oop for all safepoints.

Xin Liu has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 36 commits:

 - Merge branch 'master' into optimize_substring
 - fix regression for x86-32
   
   if LP64 is off, the offset of AddP must be I instead of L.
   x86 also doesn't emit encodeP/storeN. it use storeP instead.
 - add a statistical counter for OptimizeTempArray.
   
   -XX:+PrintOptoStatistics shows it
 - [SIM-JVM-450] support deoptimization v2
   
   because the src oop of scobj may be another scobj, deoptimization sort
   all objects in topological order.
   
   separate creation of dst oop and reassignment of it.
 - add a unit test for deoptimization
 - [SIM-JVM-450] support deoptimization part2
   
   if OptimizeTempArray eliminates an AllocateArrayNode, scalar replacement will
   create a nested SafePointScalarObjectNode for the field value:byte[] of j.l.String.
   we use the nested sobj and an ObjectValue an envelope. it consists of 3 fields:
   1. src 2. src_positio 3. length.
   
   deoptimizaton recognizes this ad-hoc ObjectValue and re-allocate an arrayOop
   for the String object.
 - enable OptimizeTempArray by default
 - Merge branch 'master' into optimize_substring
 - Revert "8260198: TypeInstPtr::dump2() emits multiple lines if Verbose is set"
   
   This reverts commit a49e34688d7d7c9d3c0d9c824d33f359613c2fc1.
 - Revert "add a new bucket afterea_late_inlines"
   
   afterea_late_inlines bucket is not useful. revert it and its relevant changes
 - ... and 26 more: https://git.openjdk.java.net/jdk/compare/849f4c0f...21693ddd

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

Changes: https://git.openjdk.java.net/jdk/pull/2570/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2570&range=01
  Stats: 861 lines in 16 files changed: 844 ins; 2 del; 15 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2570.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2570/head:pull/2570

PR: https://git.openjdk.java.net/jdk/pull/2570
Reply | Threaded
Open this post in threaded view
|

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

Volker Simonis-3
On Mon, 15 Feb 2021 19:00:57 GMT, Xin Liu <[hidden email]> wrote:

>> There are 3 nodes involving in the construction of a java.lang.String object.
>> 1. Allocate of itself, aka. alloc
>> 2. AllocateArray of a byte array, which is value:byte[], aka. aa
>> 3. ArrayCopyNode which copys in the contents of value, aka. ac
>>
>> Lemma
>> When a String object `alloc` is scalar replaced, C2 can eliminate `aa` and `ac`.
>>
>> Because `alloc` is scalar replaced, it must be non-escaped. The field value:byte[] of j.l.String cannot be seen by external world, therefore it must not be global escaped. Because the buffer is marked as stable, it is safe to assume its contents are whatever ac copies in. Because all public java.lang.String constructors clone the incoming array, the source of `ac` is stable as well.
>>
>> It is possible to rewire `aa` to the source of ac with the correct offset. That is to say, we can replace both `aa` and `ac` with a “shallow copy” of the source of `ac`. It’s safe if C2 keeps a reference of the source oop for all safepoints.
>
> Xin Liu has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 36 commits:
>
>  - Merge branch 'master' into optimize_substring
>  - fix regression for x86-32
>    
>    if LP64 is off, the offset of AddP must be I instead of L.
>    x86 also doesn't emit encodeP/storeN. it use storeP instead.
>  - add a statistical counter for OptimizeTempArray.
>    
>    -XX:+PrintOptoStatistics shows it
>  - [SIM-JVM-450] support deoptimization v2
>    
>    because the src oop of scobj may be another scobj, deoptimization sort
>    all objects in topological order.
>    
>    separate creation of dst oop and reassignment of it.
>  - add a unit test for deoptimization
>  - [SIM-JVM-450] support deoptimization part2
>    
>    if OptimizeTempArray eliminates an AllocateArrayNode, scalar replacement will
>    create a nested SafePointScalarObjectNode for the field value:byte[] of j.l.String.
>    we use the nested sobj and an ObjectValue an envelope. it consists of 3 fields:
>    1. src 2. src_positio 3. length.
>    
>    deoptimizaton recognizes this ad-hoc ObjectValue and re-allocate an arrayOop
>    for the String object.
>  - enable OptimizeTempArray by default
>  - Merge branch 'master' into optimize_substring
>  - Revert "8260198: TypeInstPtr::dump2() emits multiple lines if Verbose is set"
>    
>    This reverts commit a49e34688d7d7c9d3c0d9c824d33f359613c2fc1.
>  - Revert "add a new bucket afterea_late_inlines"
>    
>    afterea_late_inlines bucket is not useful. revert it and its relevant changes
>  - ... and 26 more: https://git.openjdk.java.net/jdk/compare/849f4c0f...21693ddd

src/hotspot/share/opto/macro.cpp line 1317:

> 1315: //
> 1316: //
> 1317: //  EncodeP:  delele because we don't need storeN

"delete" not "delele"

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

PR: https://git.openjdk.java.net/jdk/pull/2570
Reply | Threaded
Open this post in threaded view
|

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

Xin Liu
On Wed, 17 Feb 2021 17:42:48 GMT, Volker Simonis <[hidden email]> wrote:

>> Xin Liu has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 36 commits:
>>
>>  - Merge branch 'master' into optimize_substring
>>  - fix regression for x86-32
>>    
>>    if LP64 is off, the offset of AddP must be I instead of L.
>>    x86 also doesn't emit encodeP/storeN. it use storeP instead.
>>  - add a statistical counter for OptimizeTempArray.
>>    
>>    -XX:+PrintOptoStatistics shows it
>>  - [SIM-JVM-450] support deoptimization v2
>>    
>>    because the src oop of scobj may be another scobj, deoptimization sort
>>    all objects in topological order.
>>    
>>    separate creation of dst oop and reassignment of it.
>>  - add a unit test for deoptimization
>>  - [SIM-JVM-450] support deoptimization part2
>>    
>>    if OptimizeTempArray eliminates an AllocateArrayNode, scalar replacement will
>>    create a nested SafePointScalarObjectNode for the field value:byte[] of j.l.String.
>>    we use the nested sobj and an ObjectValue an envelope. it consists of 3 fields:
>>    1. src 2. src_positio 3. length.
>>    
>>    deoptimizaton recognizes this ad-hoc ObjectValue and re-allocate an arrayOop
>>    for the String object.
>>  - enable OptimizeTempArray by default
>>  - Merge branch 'master' into optimize_substring
>>  - Revert "8260198: TypeInstPtr::dump2() emits multiple lines if Verbose is set"
>>    
>>    This reverts commit a49e34688d7d7c9d3c0d9c824d33f359613c2fc1.
>>  - Revert "add a new bucket afterea_late_inlines"
>>    
>>    afterea_late_inlines bucket is not useful. revert it and its relevant changes
>>  - ... and 26 more: https://git.openjdk.java.net/jdk/compare/849f4c0f...21693ddd
>
> src/hotspot/share/opto/macro.cpp line 1317:
>
>> 1315: //
>> 1316: //
>> 1317: //  EncodeP:  delele because we don't need storeN
>
> "delete" not "delele"

got it. thanks. I will update it.

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

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