8318457: Use prefix-less prepend methods directly to reduce branches in String concat expressions

[cl4es]

In #16006 using StringConcatFactory seem to induce a run-to-run variance. One idea to reduce this is to generate expression trees with fewer branches by looking up and using the prefix-less prepend methods directly for cases where constant is null/empty so that the prefixed prepender will always deal with a non-null prefix constant.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8318457: Use prefix-less prepend methods directly to reduce branches in String concat expressions (Enhancement - P4)

Reviewers

• Jim Laskey (@JimLaskey - Reviewer) ⚠️ Review applies to 4e7dfc0f
• Chen Liang (@liach - Author) ⚠️ Review applies to faa823d3

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/16244/head:pull/16244
$ git checkout pull/16244

Update a local copy of the PR:
$ git checkout pull/16244
$ git pull https://git.openjdk.org/jdk.git pull/16244/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16244

View PR using the GUI difftool:
$ git pr show -t 16244

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/16244.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back redestad! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@cl4es The following label will be automatically applied to this pull request:

• core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 02: Full - Incremental (ca0ac91f)
• 01: Full - Incremental (faa823d3)
• 00: Full (4e7dfc0f)

[openjdk]

@cl4es This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8318457: Use prefix-less prepend methods directly to reduce branches in String concat expressions

Reviewed-by: jlaskey, liach

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 13 new commits pushed to the master branch:

• 9cf334f: 8318383: Remove duplicated checks in os::get_native_stack() in posix implementation
• defc7e0: 8318454: TestLayoutPaths broken on Big Endian platforms after JDK-8317837
• 3c70f2c: 8318418: hsdis build fails with system binutils on Ubuntu
• 15acf4b: 8318324: Drop redundant default methods from FFM API
• 1a09835: 8317358: G1: Make TestMaxNewSize use createTestJvm
• 47bb1a1: 8318415: Adjust describing comment of os_getChildren after 8315026
• 80bd22d: 8316144: Remove unused field jdk.internal.util.xml.impl.XMLStreamWriterImpl.Element._Depth
• c0e154c: 8318089: Class space not marked as such with NMT when CDS is off
• 24bc5bd: 8318104: macOS 10.13 check in TabButtonAccessibility.m can be removed
• e25a49a: 8318471: ProblemList compiler/sharedstubs/SharedTrampolineTest.java
• ... and 3 more: https://git.openjdk.org/jdk/compare/6fc35142315f1616fa35e415005c9483939c6920...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[cl4es]

Thanks for the quick review, @JimLaskey! I'll await tier1-3 test results before integration.

[cl4es]

Using dropLookupMode caused a bootstrap cycle in tests where a SecurityManager is installed, causing lots of test failures. Reverting that part.

[liach]

An alternative way to provide access control could be to call MethodHandles.lookup() in the anonymous JavaLangAccess implementation in System. This won't require an SM check as the lookup has full-privilege access, yet the lookup cannot access private members of StringConcatHelper.

[cl4es]

I think we try to avoid having any additional logic in the JavaLangAccess type proxies. Either way I think this is getting out of scope for this PR and if there's a real need to lock down the mechanism it can be done in a follow-up. Do you agree?

[liach]

I strongly agree. SharedSecrets access implementations strive to be as concise as possible. We can always revisit the access control part.

[cl4es]

/integrate

[openjdk]

Going to push as commit fe52917.
Since your change was applied there have been 37 commits pushed to the master branch:

• 71c99a0: 8318448: Link PopupMenu/PopupMenuLocation.java failure to JDK-8259913
• 2c23391: 8318101: Additional test cases for CSSAttributeEqualityBug
• deadb9c: 8304684: Memory leak in DirectivesParser::set_option_flag
• a03767c: 8318049: C2: assert(!failure) failed: Missed optimization opportunity in PhaseIterGVN
• 848ecc1: 8318538: Add a way to obtain a strided var handle from a layout
• b07da3a: 8317819: Scope should reflect lifetime of underying resource (mainline)
• 6f1d896: 8318510: Serial: Remove TenuredGeneration::block_size
• 8f4ebd8: 8317920: JDWP-agent sends broken exception event with onthrow option
• cd25d1a: 8318296: Move Space::initialize to ContiguousSpace
• 744f206: 8318525: Atomic gtest should run as TEST_VM to access VM capabilities
• ... and 27 more: https://git.openjdk.org/jdk/compare/6fc35142315f1616fa35e415005c9483939c6920...master

Your commit was automatically rebased without conflicts.

[openjdk]

@cl4es Pushed as commit fe52917.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.