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

8275872: Sync J2DBench run and analyze Makefile targets with build.xml #6068

Closed

Conversation

judovana
Copy link
Contributor

@judovana judovana commented Oct 21, 2021

The run targets of makefile to run J2DBench.jar/J2DAnalyzer.jar were
lacking the dist directory. This patch is fixing it. Note, that
build.xml have correct paths


Progress

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

Issue

  • JDK-8275872: Sync J2DBench run and analyze Makefile targets with build.xml

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 6068

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

Using diff file

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

@judovana
Copy link
Contributor Author

@judovana judovana commented Oct 21, 2021

I'm looking for kind soul to create bug and merge on my behalf.

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Oct 21, 2021

👋 Welcome back jvanek! 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 openjdk bot commented Oct 21, 2021

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

  • client

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.

@openjdk openjdk bot added the client label Oct 21, 2021
@judovana
Copy link
Contributor Author

@judovana judovana commented Oct 21, 2021

/label rfr

@judovana
Copy link
Contributor Author

@judovana judovana commented Oct 21, 2021

/label add rfr

@openjdk
Copy link

@openjdk openjdk bot commented Oct 21, 2021

@judovana The label rfr is not a valid label. These labels are valid:

  • serviceability
  • hotspot
  • hotspot-compiler
  • ide-support
  • kulla
  • i18n
  • shenandoah
  • jdk
  • javadoc
  • security
  • hotspot-runtime
  • jmx
  • build
  • nio
  • client
  • core-libs
  • compiler
  • net
  • hotspot-gc
  • hotspot-jfr

@openjdk
Copy link

@openjdk openjdk bot commented Oct 21, 2021

@judovana The label rfr is not a valid label. These labels are valid:

  • serviceability
  • hotspot
  • hotspot-compiler
  • ide-support
  • kulla
  • i18n
  • shenandoah
  • jdk
  • javadoc
  • security
  • hotspot-runtime
  • jmx
  • build
  • nio
  • client
  • core-libs
  • compiler
  • net
  • hotspot-gc
  • hotspot-jfr

@magicus
Copy link
Member

@magicus magicus commented Oct 21, 2021

/label build

@magicus
Copy link
Member

@magicus magicus commented Oct 21, 2021

Hi @judovana

This PR is not going to be noticed by anyone who can review it. Please send an email to the relevant mailing lists (in this case client-dev@openjdk.java.net and build-dev@openjdk.java.net) to raise a discussion and, hopefully, have someone sponsor this PR.

@openjdk openjdk bot added the build label Oct 21, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Oct 21, 2021

@magicus
The build label was successfully added.

@mrserb
Copy link
Member

@mrserb mrserb commented Oct 23, 2021

this looks fine, please update the copyright date

@gnu-andrew
Copy link
Member

@gnu-andrew gnu-andrew commented Oct 25, 2021

I've filed https://bugs.openjdk.java.net/browse/JDK-8275872 and am happy to sponsor this minor change.
@judovana, please update the summary with the bug ID. Thanks.

@gnu-andrew
Copy link
Member

@gnu-andrew gnu-andrew commented Oct 25, 2021

/summary 8275872: Sync J2DBench run and analyze Makefile targets with build.xml

@openjdk
Copy link

@openjdk openjdk bot commented Oct 25, 2021

@gnu-andrew Only the author (@judovana) is allowed to issue the /summary command.

@judovana judovana force-pushed the j2dbenchBadRuntimeArtifacts branch from 63ba355 to e28f550 Compare Oct 25, 2021
@judovana judovana changed the title Fixed path for run of J2DBench.jar and J2DAnalyzer.jar 8275872: Sync J2DBench run and analyze Makefile targets with build.xml Oct 25, 2021
@judovana
Copy link
Contributor Author

@judovana judovana commented Oct 25, 2021

/summary 8275872: Sync J2DBench run and analyze Makefile targets with build.xml

@openjdk
Copy link

@openjdk openjdk bot commented Oct 25, 2021

@judovana Setting summary to 8275872: Sync J2DBench run and analyze Makefile targets with build.xml

@judovana
Copy link
Contributor Author

@judovana judovana commented Oct 25, 2021

@mrserb @gnu-andrew hi! Thanx! The PR was amended.

@openjdk openjdk bot added the rfr label Oct 25, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Oct 25, 2021

Webrevs

@@ -1,5 +1,5 @@
#
# Copyright (c) 2002, 2020, Oracle and/or its affiliates. All rights reserved.
# Copyright (c) 2002, 2020, 2021 Oracle and/or its affiliates. All rights reserved.
Copy link
Member

@mrserb mrserb Oct 25, 2021

Choose a reason for hiding this comment

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

You need to update the second date, no need to create an additional one.
The first date is when the file was created and the second is when it was updated last time.

Copy link
Contributor Author

@judovana judovana Oct 26, 2021

Choose a reason for hiding this comment

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

Oh damn. Sorry. Fixed

Fixed path for run of J2DBench.jar and J2DAnalyzer.jar
The run targets of makefile to run J2DBench.jar/J2DAnalyzer.jar were
lacking the dist directory. This patch is fixing it. Note, that
build.xml have correct paths
@judovana judovana force-pushed the j2dbenchBadRuntimeArtifacts branch from e28f550 to 811f18d Compare Oct 26, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Oct 26, 2021

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

8275872: Sync J2DBench run and analyze Makefile targets with build.xml

Reviewed-by: ihse, andrew

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

  • 19f76c2: 8275079: Remove unnecessary conversion to String in java.net.http
  • e5cd269: 8274944: AppCDS dump causes SEGV in VM thread while adjusting lambda proxy class info
  • 82f4aac: 8259609: C2: optimize long range checks in long counted loops
  • 574f890: 8275720: CommonComponentAccessibility.createWithParent isWrapped causes mem leak
  • 7c88a59: 8275809: crash in [CommonComponentAccessibility getCAccessible:withEnv:]
  • c9dec2f: 8273299: Unnecessary Vector usage in java.security.jgss
  • b98ed55: 8275819: [TableRowAccessibility accessibilityChildren] method is ineffective
  • 71d593e: 8275162: Use varargs in 'def' macros in mutexLocker.cpp
  • 7ca053d: 8251904: vmTestbase/nsk/sysdict/vm/stress/btree/btree010/btree010.java fails with ClassNotFoundException: nsk.sysdict.share.BTree0LLRLRLRRLR
  • 4be88d5: 8275047: Optimize existing fill stubs for AVX-512 target
  • ... and 35 more: https://git.openjdk.java.net/jdk/compare/bef8cf1ba14d3846977942844f341f5c5a1f44c4...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 (@magicus, @gnu-andrew) 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 Oct 26, 2021
@magicus
Copy link
Member

@magicus magicus commented Oct 26, 2021

@judovana Unfortunately you got "tricked" by @gnu-andrew 's comment into issuing a /summary comment, when you shouldn't have. That only makes the bug title be repeated as a summary. This is not how summaries are intented. (In fact, they are hardly used at all in the JDK project, and only to shortly describe the fix if the bug title is not descriptive enough).

I'm not sure if you can remove a summary, but I think you can by just issuing a blank /summary command, without any contents.

@magicus
Copy link
Member

@magicus magicus commented Oct 26, 2021

I can sponsor the PR once this is done.

@judovana
Copy link
Contributor Author

@judovana judovana commented Oct 26, 2021

/summary

@openjdk
Copy link

@openjdk openjdk bot commented Oct 26, 2021

@judovana Removing existing summary

@judovana
Copy link
Contributor Author

@judovana judovana commented Oct 26, 2021

Attmept done. Looks like the summary is there already once. Thanx again.

@dholmes-ora
Copy link
Member

@dholmes-ora dholmes-ora commented Oct 26, 2021

@judovana please do not use "force push" in a PR as it makes the review process very difficult and can leave orphaned comments. You can simply push additional changes after the initial commit. The skara tooling with flatten all commits into one single, properly formulated, commit that will actually be pushed to the OpenJDK repo. Thanks.

@magicus
Copy link
Member

@magicus magicus commented Oct 26, 2021

/sponsor

@openjdk
Copy link

@openjdk openjdk bot commented Oct 26, 2021

@magicus The change author (@judovana) must issue an integrate command before the integration can be sponsored.

@judovana
Copy link
Contributor Author

@judovana judovana commented Oct 26, 2021

thanx @dholmes-ora , will do

@judovana
Copy link
Contributor Author

@judovana judovana commented Oct 26, 2021

/integrate

@openjdk openjdk bot added the sponsor label Oct 26, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Oct 26, 2021

@judovana
Your change (at version 811f18d) is now ready to be sponsored by a Committer.

@magicus
Copy link
Member

@magicus magicus commented Oct 26, 2021

/sponsor

@openjdk
Copy link

@openjdk openjdk bot commented Oct 26, 2021

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

  • 19f76c2: 8275079: Remove unnecessary conversion to String in java.net.http
  • e5cd269: 8274944: AppCDS dump causes SEGV in VM thread while adjusting lambda proxy class info
  • 82f4aac: 8259609: C2: optimize long range checks in long counted loops
  • 574f890: 8275720: CommonComponentAccessibility.createWithParent isWrapped causes mem leak
  • 7c88a59: 8275809: crash in [CommonComponentAccessibility getCAccessible:withEnv:]
  • c9dec2f: 8273299: Unnecessary Vector usage in java.security.jgss
  • b98ed55: 8275819: [TableRowAccessibility accessibilityChildren] method is ineffective
  • 71d593e: 8275162: Use varargs in 'def' macros in mutexLocker.cpp
  • 7ca053d: 8251904: vmTestbase/nsk/sysdict/vm/stress/btree/btree010/btree010.java fails with ClassNotFoundException: nsk.sysdict.share.BTree0LLRLRLRRLR
  • 4be88d5: 8275047: Optimize existing fill stubs for AVX-512 target
  • ... and 35 more: https://git.openjdk.java.net/jdk/compare/bef8cf1ba14d3846977942844f341f5c5a1f44c4...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot closed this Oct 26, 2021
@openjdk openjdk bot added integrated and removed ready rfr sponsor labels Oct 26, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Oct 26, 2021

@magicus @judovana Pushed as commit f1f5e26.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build client integrated
5 participants