8349991: GraphUtils.java can use String.replace() instead of String.replaceAll()

[archiecobbs]

This is a tiny performance improvement that replaces a few invocations of String.replaceAll() with equivalent invocations of String.replace(char, char) and avoids the unnecessary use of regular expressions.

---

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-8349991: GraphUtils.java can use String.replace() instead of String.replaceAll() (Bug - P4)

Reviewers

• Maurizio Cimadamore (@mcimadamore - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23658

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back acobbs! 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]

@archiecobbs 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:

8349991: GraphUtils.java can use String.replace() instead of String.replaceAll()

Reviewed-by: mcimadamore

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 116 new commits pushed to the master branch:

• 65f79c1: 8347335: ZGC: Use limitless mark stack memory
• e410af0: 8342393: Promote commutative vector IR node sharing
• f755fad: 8349653: Clarify the docs for MemorySegment::reinterpret
• a5c9a4d: 8349032: C2: Parse Predicate refactoring in Loop Unswitching broke fix for JDK-8290850
• 302bed0: 8350499: Minimal build fails with slowdebug builds
• 0795d11: 8350464: The flags to set the native priority for the VMThread and Java threads need a broader range
• 05b4812: 8350041: Skip test/jdk/java/lang/String/nativeEncoding/StringPlatformChars.java on static JDK
• a891630: 8350480: RISC-V: Relax assertion about registers in C2_MacroAssembler::minmax_fp
• 5cbd9d1: 8349959: Test CR6740048.java passes unexpectedly missing CR6740048.xsd
• 25322aa: 8350258: AArch64: Client build fails after JDK-8347917
• ... and 106 more: https://git.openjdk.org/jdk/compare/ba28119642a7cfa850707a01ce8e589c3a80d416...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.

[openjdk]

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

• compiler

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

• 00: Full (d2d9b6a6)

[mcimadamore]

can't there be more than one , in the toString of Properties object?

[mcimadamore]

I know that javac only ever uses two properties at most... but this class is rather general, and the proposed change would introduce subtle bugs if a third property is ever added to inference nodes -- or any other node using in javac implementation

[mcimadamore]

(separately, I'm also curious of when such a change makes a real impact -- toDot() is mostly used when debugging... but I'm ok with replacing complex methods with simpler forms -- where possible)

[efge]

But replace replaces all occurrences already.

[mcimadamore]

But replace replaces all occurrences already.

Ah yes - that's where I got confused - thanks. (replace vs. replaceAll vs. replaceFirst mix a number of different concerns - such as one vs. many and string vs. char).

[archiecobbs]

Thanks for the review!

[archiecobbs]

/integrate

[openjdk]

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

• d422abc: 8350668: has_extra_module_paths in filemap.cpp may be uninitialized
• 829d7a8: 8339889: Several compiler tests ignore vm flags and not marked as flagless
• 0151b15: 8348367: Remove hotspot_not_fast_compiler and hotspot_slow_compiler test groups
• c5992ca: 8349533: Refactor validator tests shell files to java
• dea7a9f: 8350516: Update model numbers for ECore-based cpus
• 5023971: 8267068: Incomplete @throws javadoc for various javax.crypto.spec classes
• 08bc59d: 8138614: (spec str) StringBuffer and StringBuilder methods improperly require "new" String to be returned
• b17c0b6: 8350585: InlineSecondarySupersTest must be guarded on ppc64 by COMPILER2
• 62f39bd: 8350636: Potential null-pointer dereference in MallocSiteTable::new_entry
• cfeb7d6: 8350497: os::create_thread unify init thread attributes part across UNIX platforms
• ... and 127 more: https://git.openjdk.org/jdk/compare/ba28119642a7cfa850707a01ce8e589c3a80d416...master

Your commit was automatically rebased without conflicts.

[openjdk]

@archiecobbs Pushed as commit a3188e0.

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