Skip to content

Conversation

@lgxbslgx
Copy link
Member

@lgxbslgx lgxbslgx commented May 7, 2021

Hi all,

Currently, the class com.sun.tools.javac.util.IntHashTable exposes methods hash(Object key) and lookup(Object key) to users. It is not good because they are internal methods to achieve the features.

On the other hand, the index of the mapping (key or value) is the internal data struct. The users should not know or use it actually. So the parameters of the following methods should be revised.

  1. lookup(Object key, int hash) -> lookup(Object key)
  2. getFromIndex(int index) -> get(Object key)
  3. putAtIndex(Object key, int value, int index) -> put(Object key, int value)

And the corresponding places where they are used need to be adjusted too.

The patch optimizes them, adds some comments and cleans the code.
Thank you for taking the time to review.

Best Regards,
-- Guoxiong


Progress

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

Issue

  • JDK-8266675: Optimize IntHashTable for encapsulation and ease of use

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3912

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

Using diff file

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

@bridgekeeper
Copy link

bridgekeeper bot commented May 7, 2021

👋 Welcome back gli! 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
Copy link

openjdk bot commented May 7, 2021

@lgxbslgx 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.

@lgxbslgx lgxbslgx changed the title Jdk 8266675 8266675: Optimize IntHashTable for encapsulation and ease of use May 7, 2021
@openjdk openjdk bot added compiler compiler-dev@openjdk.org rfr Pull request is ready for review labels May 7, 2021
@mlbridge
Copy link

mlbridge bot commented May 7, 2021

Webrevs

Copy link
Contributor

@mcimadamore mcimadamore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@openjdk
Copy link

openjdk bot commented May 7, 2021

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

8266675: Optimize IntHashTable for encapsulation and ease of use

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

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 (@mcimadamore) 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 Pull request is ready to be integrated label May 7, 2021
@lgxbslgx
Copy link
Member Author

lgxbslgx commented May 7, 2021

@mcimadamore Thanks for your review. Could I get your help to sponsor this patch?

/integrate

@openjdk openjdk bot added the sponsor Pull request is ready to be sponsored label May 7, 2021
@openjdk
Copy link

openjdk bot commented May 7, 2021

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

@vicente-romero-oracle
Copy link
Contributor

/sponsor

@openjdk openjdk bot closed this May 8, 2021
@openjdk openjdk bot added integrated Pull request has been integrated and removed sponsor Pull request is ready to be sponsored ready Pull request is ready to be integrated rfr Pull request is ready for review labels May 8, 2021
@openjdk
Copy link

openjdk bot commented May 8, 2021

@vicente-romero-oracle @lgxbslgx Since your change was applied there have been 22 commits pushed to the master branch:

  • 04fad70: 8266765: [BACKOUT] JDK-8255493 Support for pre-generated java.lang.invoke classes in CDS dynamic archive
  • 0790e60: 8196743: jstatd doesn't see new Java processes inside Docker container
  • c6aa8f1: 8232644: bugs in serialized-form.html
  • b5b3119: 8266589: (fs) Improve performance of Files.copy() on macOS using copyfile(3)
  • 947d69d: 8265042: javadoc HTML files not generated for types nested in records
  • 946b0fe: 8266645: javac should not check for sealed supertypes in intersection types
  • 74fecc0: 8266503: [UL] Make Decorations safely copy-able and reduce their size
  • 86b8dc9: 8265426: Update java.security to use instanceof pattern variable
  • 3fcdc50: 8266646: Add more diagnostic to java/lang/System/LoggerFinder/modules
  • 9a19a0c: 8264760: JVM crashes when two threads encounter the same resolution error
  • ... and 12 more: https://git.openjdk.java.net/jdk/compare/36e5ad61e63e2f1da9cf565c607db28f23622ea9...master

Your commit was automatically rebased without conflicts.

Pushed as commit ff77ca8.

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

@lgxbslgx lgxbslgx deleted the JDK-8266675 branch May 8, 2021 03:18
@mcimadamore
Copy link
Contributor

/sponsor

@openjdk
Copy link

openjdk bot commented May 10, 2021

@mcimadamore The command sponsor can only be used in open pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler compiler-dev@openjdk.org integrated Pull request has been integrated

Development

Successfully merging this pull request may close these issues.

3 participants