Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8273140: Replace usages of Enum.class.getEnumConstants() with Enum.values() where possible #5303

Closed
wants to merge 4 commits into from

Conversation

@stsypanov
Copy link
Contributor

@stsypanov stsypanov commented Aug 30, 2021

Just a very tiny clean-up.

There are some places in JDK code base where we call Enum.class.getEnumConstants() to get all the values of the referenced enum. This is excessive, less-readable and slower than just calling Enum.values() as in getEnumConstants() we have volatile access:

public T[] getEnumConstants() {
    T[] values = getEnumConstantsShared();
    return (values != null) ? values.clone() : null;
}

private transient volatile T[] enumConstants;

T[] getEnumConstantsShared() {
    T[] constants = enumConstants;
    if (constants == null) { /* ... */ }
    return constants;
}

Calling values() method is slightly faster:

@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
public class EnumBenchmark {

  @Benchmark
  public Enum[] values() {
    return Enum.values();
  }

  @Benchmark
  public Enum[] getEnumConstants() {
    return Enum.class.getEnumConstants();
  }

  private enum Enum {
    A,
    B
  }
}
Benchmark                                                        Mode  Cnt     Score     Error   Units
EnumBenchmark.getEnumConstants                                   avgt   15     6,265 ±   0,051   ns/op
EnumBenchmark.getEnumConstants:·gc.alloc.rate                    avgt   15  2434,075 ±  19,568  MB/sec
EnumBenchmark.getEnumConstants:·gc.alloc.rate.norm               avgt   15    24,002 ±   0,001    B/op
EnumBenchmark.getEnumConstants:·gc.churn.G1_Eden_Space           avgt   15  2433,709 ±  70,216  MB/sec
EnumBenchmark.getEnumConstants:·gc.churn.G1_Eden_Space.norm      avgt   15    23,998 ±   0,659    B/op
EnumBenchmark.getEnumConstants:·gc.churn.G1_Survivor_Space       avgt   15     0,009 ±   0,003  MB/sec
EnumBenchmark.getEnumConstants:·gc.churn.G1_Survivor_Space.norm  avgt   15    ≈ 10⁻⁴              B/op
EnumBenchmark.getEnumConstants:·gc.count                         avgt   15   210,000            counts
EnumBenchmark.getEnumConstants:·gc.time                          avgt   15   119,000                ms

EnumBenchmark.values                                             avgt   15     4,164 ±   0,134   ns/op
EnumBenchmark.values:·gc.alloc.rate                              avgt   15  3665,341 ± 120,721  MB/sec
EnumBenchmark.values:·gc.alloc.rate.norm                         avgt   15    24,002 ±   0,001    B/op
EnumBenchmark.values:·gc.churn.G1_Eden_Space                     avgt   15  3660,512 ± 137,250  MB/sec
EnumBenchmark.values:·gc.churn.G1_Eden_Space.norm                avgt   15    23,972 ±   0,529    B/op
EnumBenchmark.values:·gc.churn.G1_Survivor_Space                 avgt   15     0,017 ±   0,003  MB/sec
EnumBenchmark.values:·gc.churn.G1_Survivor_Space.norm            avgt   15    ≈ 10⁻⁴              B/op
EnumBenchmark.values:·gc.count                                   avgt   15   262,000            counts
EnumBenchmark.values:·gc.time                                    avgt   15   155,000                ms

Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8273140: Replace usages of Enum.class.getEnumConstants() with Enum.values() where possible

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5303/head:pull/5303
$ git checkout pull/5303

Update a local copy of the PR:
$ git checkout pull/5303
$ git pull https://git.openjdk.java.net/jdk pull/5303/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5303

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5303.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Aug 30, 2021

👋 Welcome back stsypanov! 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 openjdk bot added the rfr label Aug 30, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Aug 30, 2021

@stsypanov The following labels will be automatically applied to this pull request:

  • 2d
  • core-libs
  • hotspot-gc

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

@mlbridge
Copy link

@mlbridge mlbridge bot commented Aug 30, 2021

Copy link
Member

@kevinrushforth kevinrushforth left a comment

You missed two of the copyright year problems.

src/java.desktop/share/classes/sun/font/EAttribute.java Outdated Show resolved Hide resolved
src/java.sql/share/classes/java/sql/JDBCType.java Outdated Show resolved Hide resolved
Copy link
Contributor

@tschatzl tschatzl left a comment

Lgtm.

@openjdk
Copy link

@openjdk openjdk bot commented Sep 1, 2021

⚠️ @stsypanov the full name on your profile does not match the author name in this pull requests' HEAD commit. If this pull request gets integrated then the author name from this pull requests' HEAD commit will be used for the resulting commit. If you wish to push a new commit with a different author name, then please run the following commands in a local repository of your personal fork:

$ git checkout 8273140
$ git commit -c user.name='Preferred Full Name' --allow-empty -m 'Update full name'
$ git push

@openjdk
Copy link

@openjdk openjdk bot commented Sep 1, 2021

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

8273140: Replace usages of Enum.class.getEnumConstants() with Enum.values() where possible

Reviewed-by: tschatzl

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

  • 0e14bf7: 8273176: handle latest VS2019 in abstract_vm_version
  • f1c5e26: 8273206: jdk/jfr/event/gc/collection/TestG1ParallelPhases.java fails after JDK-8159979
  • e600fe1: 8272618: Unnecessary Attr.visitIdent.noOuterThisPath
  • 2fce7cb: 8272963: Update the java manpage markdown source
  • 18a731a: 8269770: nsk tests should start IOPipe channel before launch debuggee - Debugee.prepareDebugee
  • 9c392d0: 8273197: ProblemList 2 jtools tests due to JDK-8273187
  • 3d657eb: 8262186: Call X509KeyManager.chooseClientAlias once for all key types
  • c1e0aac: 8273186: Remove leftover comment about sparse remembered set in G1 HeapRegionRemSet
  • 683e30d: 8273169: java/util/regex/NegativeArraySize.java failed after JDK-8271302
  • 1996f64: 8273092: Sort classlist in JDK image
  • ... and 17 more: https://git.openjdk.java.net/jdk/compare/fbffa54efe447a4c911af2be1d7774a8c60d6ede...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@tschatzl) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added the ready label Sep 1, 2021
@stsypanov
Copy link
Contributor Author

@stsypanov stsypanov commented Sep 1, 2021

/integrate

@openjdk openjdk bot added the sponsor label Sep 1, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Sep 1, 2021

@stsypanov
Your change (at version a5f58fe) is now ready to be sponsored by a Committer.

@tschatzl
Copy link
Contributor

@tschatzl tschatzl commented Sep 2, 2021

/sponsor

@openjdk
Copy link

@openjdk openjdk bot commented Sep 2, 2021

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

  • 857a930: 8263375: Support stack watermarks in Zero VM
  • 6cfe314: 8272970: Parallelize runtime/InvocationTests/
  • a9a83b2: 8273256: runtime/cds/appcds/TestEpsilonGCWithCDS.java fails due to Unrecognized VM option 'ObjectAlignmentInBytes=64' on x86_32
  • 1a5a2b6: 8271745: Correct block size for KW,KWP mode and use fixed IV for KWP mode for SunJCE
  • 2f01a6f: 8273157: Add convenience methods to Messager
  • 9689f61: 8272788: Nonleaf ranked locks should not be safepoint_check_never
  • 4ee0dac: 8273248: ProblemList java/lang/instrument/BootClassPath/BootClassPathTest.sh on all configs
  • 655ea6d: 8270489: Support archived heap objects in EpsilonGC
  • dacd197: 8273217: Make ParHeapInspectTask _safepoint_check_never
  • 02822e1: 8272377: assert preconditions that are ensured when created in add_final_edges
  • ... and 28 more: https://git.openjdk.java.net/jdk/compare/fbffa54efe447a4c911af2be1d7774a8c60d6ede...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot closed this Sep 2, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Sep 2, 2021

@tschatzl @stsypanov Pushed as commit 152e669.

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

@stsypanov stsypanov deleted the 8273140 branch Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants