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

8261302: NMT: Improve malloc site table hashing #2608

Closed
wants to merge 1 commit into from

Conversation

Delawen
Copy link
Contributor

@Delawen Delawen commented Mar 18, 2024

This is the same fix that was applied in openjdk/jdk@a3d6e371 for https://bugs.openjdk.org/browse/JDK-8261302

It simplifies the way of calculating the hash of a stack.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • JDK-8261302 needs maintainer approval

Issue

  • JDK-8261302: NMT: Improve malloc site table hashing (Enhancement - P4 - Rejected)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev.git pull/2608/head:pull/2608
$ git checkout pull/2608

Update a local copy of the PR:
$ git checkout pull/2608
$ git pull https://git.openjdk.org/jdk11u-dev.git pull/2608/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2608

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/2608.diff

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Mar 18, 2024

👋 Welcome back Delawen! 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 Mar 18, 2024

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@openjdk openjdk bot changed the title Backport a3d6e37153d09b3198c983bd7b689dca570386dc 8261302: NMT: Improve malloc site table hashing Mar 18, 2024
@openjdk
Copy link

openjdk bot commented Mar 18, 2024

This backport pull request has now been updated with issue from the original commit.

@openjdk
Copy link

openjdk bot commented Mar 18, 2024

⚠️ @Delawen This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

@openjdk openjdk bot added the rfr Pull request is ready for review label Mar 18, 2024
@mlbridge
Copy link

mlbridge bot commented Mar 18, 2024

Webrevs

Copy link
Member

@tstuefe tstuefe left a comment

Choose a reason for hiding this comment

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

Good. Please note that this also needs JDK-8261520 as a follow-up.

@Delawen
Copy link
Contributor Author

Delawen commented Mar 19, 2024

/approval JDK-8261302 request Fix Request
This backport simplifies the way of calculating hashes for stacks.

@openjdk
Copy link

openjdk bot commented Mar 19, 2024

@Delawen
JDK-8261302: The approval request has been created successfully.

@openjdk openjdk bot added the approval label Mar 19, 2024
@GoeLin
Copy link
Member

GoeLin commented Mar 20, 2024

Hi @Delawen, I don't see why this should be backported to a stable release as 11.

@tstuefe
Copy link
Member

tstuefe commented Mar 20, 2024

Hi @Delawen, I don't see why this should be backported to a stable release as 11.

Hi @GoeLin , this is part of a (smallish) chain of changes that would make NMT in 11 feature-equivalent to 21. Since NMT continues to be very important to us, and since the changes in general are low-risk, I suggested making these backports.

@Delawen
Copy link
Contributor Author

Delawen commented Mar 20, 2024

I have a list of potential NMT improvements (following @tstuefe steps) that could be backported here. To me it makes sense to be as similar as JDK21, but of course this is not my call, so let me know if I should proceed.

@GoeLin
Copy link
Member

GoeLin commented Mar 21, 2024

@Delawen, @Stuefe, can you please share the list of intended backports?

@Delawen
Copy link
Contributor Author

Delawen commented Mar 21, 2024

Here is the short version list @GoeLin

To JDK11:

The one with the opened PR with its follow-up:
* JDK-8261302: NMT: Improve malloc site table hashing

Other related issues:

To JDK17 and below:

Maybe I should start with the ones associated with JDK17?

@GoeLin
Copy link
Member

GoeLin commented Mar 22, 2024

Hi @Delawen, could you please edit the comment and add links to the bugIDs?
You should start with 21.

@Delawen
Copy link
Contributor Author

Delawen commented Mar 22, 2024

Starting with 21 then!

Comment edited.

@GoeLin
Copy link
Member

GoeLin commented Mar 26, 2024

Hi @Delawen, after checking with the fellow maintainers we decided this is too much for 11.
Thomas agreed, too. Thanks for engaging in OpenJDK anyways!

@openjdk openjdk bot removed the approval label Mar 26, 2024
@Delawen
Copy link
Contributor Author

Delawen commented Mar 27, 2024

No problem, I will focus on newer versions of OpenJDK :)

@Delawen Delawen closed this Mar 27, 2024
@Delawen Delawen deleted the backport-JDK-8261302 branch March 27, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport clean rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

3 participants