8255298: Remove SurvivorAlignmentInBytes functionality #838

tschatzl · 2020-10-23T15:16:57Z

Hi all,

can I have reviews to remove the SurvivorAlignmentInBytes functionality? It has not been in use for a long time if ever, and can be removed. Searching the web also indicates that apart from the usual lists of all options and CRs it is never mentioned.

SurvivorAlignmentInBytes is an experimental option so no further process is required.

Testing: tier1-5

Thanks,
Thomas

Progress

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

Issue

JDK-8255298: Remove SurvivorAlignmentInBytes functionality

Reviewers

Aleksey Shipilev (@shipilev - Reviewer) ⚠️ Review applies to 878cac2
Albert Mingkun Yang (@albertnetymk - Author) ⚠️ Review applies to b1407f0
Kim Barrett (@kimbarrett - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/838/head:pull/838
$ git checkout pull/838

bridgekeeper · 2020-10-23T15:20:19Z

👋 Welcome back tschatzl! 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 · 2020-10-23T15:26:30Z

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

hotspot

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 · 2020-10-23T15:29:41Z

Webrevs

shipilev

Nifty cleanup! One nit I see:

shipilev · 2020-10-23T15:50:09Z

src/hotspot/share/gc/parallel/psPromotionLAB.inline.hpp

@@ -32,7 +32,7 @@
 HeapWord* PSYoungPromotionLAB::allocate(size_t size) {
  // Can't assert this, when young fills, we keep the LAB around, but flushed.
  // assert(_state != flushed, "Sanity");
-  HeapWord* obj = CollectedHeap::align_allocation_or_fail(top(), end(), SurvivorAlignmentInBytes);
+  HeapWord* obj = top();
  if (obj == NULL) {


obj cannot be NULL now? Thus the following null-check is redundant?

tschatzl · 2020-10-23T17:20:41Z

Done, thanks for catching this.

shipilev

These look fine to me.

openjdk · 2020-10-23T17:23:38Z

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

8255298: Remove SurvivorAlignmentInBytes functionality

Reviewed-by: shade, ayang, kbarrett

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

2c9dfc7: 8255234: ZGC: Bulk allocate forwarding data structures
b7d483c: 8255245: C1: Fix output of -XX:+PrintCFGToFile to open it with visualizer
5e0a8cd: 8255488: Re-enable some problem listed tests
3f8bd92: 8233556: [TESTBUG] JPopupMenu tests fail on MacOS
4acf634: 8171998: javax/swing/JMenu/4692443/bug4692443.java fails on Windows
a804c6a: 8254871: Remove unnecessary string copy in NetworkInterface.c
acd0e25: 8255254: Split os::reserve_memory and os::map_memory_to_file interfaces
dc85a3f: 8015602: [macosx] Test javax/swing/SpringLayout/4726194/bug4726194.java fails on MacOSX
1d245c6: 8252117: com/sun/jdi/BadHandshakeTest.java failed with "ConnectException: Connection refused: connect"
7a7ce02: 8007632: DES/3DES keys support in PKCS12 keystore
... and 54 more: https://git.openjdk.java.net/jdk/compare/c1524c59adb8d150aaedd0552fae0444dd2e638f...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.

kimbarrett · 2020-10-26T08:24:53Z

src/hotspot/share/gc/parallel/psPromotionLAB.inline.hpp

  HeapWord* new_top = obj + size;
  // The 'new_top>obj' check is needed to detect overflow of obj+size.
  if (new_top > obj && new_top <= end()) {
    set_top(new_top);
-    assert(is_aligned(obj, SurvivorAlignmentInBytes) && is_object_aligned(new_top),
-           "checking alignment");
+    assert(is_object_aligned(new_top), "checking alignment");
    return obj;
  } else {
    set_top(obj);


I think we don't need this set_top anymore.

tschatzl · 2020-10-27T09:45:47Z

@kimbarrett : fixed in new revision. Thanks!

tschatzl · 2020-10-29T08:46:33Z

Thanks @kstefanj @kimbarrett @shipilev for your reviews.

/integrate

openjdk · 2020-10-29T08:51:28Z

@tschatzl Since your change was applied there have been 89 commits pushed to the master branch:

4031cb4: 8254189: Improve comments for StackOverFlow and fix in_xxx() functions
caec8d2: 8233560: [TESTBUG] ToolTipManager/Test6256140.java is failing on macos
a5b42ec: 8233570: [TESTBUG] HTMLEditorKit test bug5043626.java is failing on macos
7e305ad: 8255405: sun/net/ftp/imp/FtpClient uses SimpleDateFormat in not thread-safe manner
d82a6dc: 8255438: [Vector API] More instructs in x86.ad should use legacy mode for code-gen
1a5e6c9: 8253101: Clean up CallStaticJavaNode EA flags
a7595b2: 8250669: Running JMH micros is broken after JDK-8248135
edd1988: 8255530: Additional cleanup after JDK-8235710 (elliptic curve removal)
790d6e2: 8255533: Incorrect javadoc in DateTimeFormatterBuilder.appendPattern() for 'uu'/'yy'
3f20612: 8255555: Bad copyright headers in SocketChannelCompare.java SocketChannelConnectionSetup.java UnixSocketChannelReadWrite.java
... and 79 more: https://git.openjdk.java.net/jdk/compare/c1524c59adb8d150aaedd0552fae0444dd2e638f...master

Your commit was automatically rebased without conflicts.

Pushed as commit 38574d5.

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

Initial commit

b1407f0

openjdk bot added the rfr Pull request is ready for review label Oct 23, 2020

openjdk bot added the hotspot hotspot-dev@openjdk.org label Oct 23, 2020

shipilev suggested changes Oct 23, 2020

View reviewed changes

shade review

878cac2

shipilev approved these changes Oct 23, 2020

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 23, 2020

albertnetymk approved these changes Oct 26, 2020

View reviewed changes

kimbarrett reviewed Oct 26, 2020

View reviewed changes

kbarrett review

fa947b7

kimbarrett approved these changes Oct 27, 2020

View reviewed changes

openjdk bot closed this Oct 29, 2020

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Oct 29, 2020

tschatzl deleted the 8255298-remove-survivoralignmentinbytes branch November 2, 2020 10:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8255298: Remove SurvivorAlignmentInBytes functionality #838

8255298: Remove SurvivorAlignmentInBytes functionality #838

tschatzl commented Oct 23, 2020 •

edited by openjdk bot

Loading

bridgekeeper bot commented Oct 23, 2020

openjdk bot commented Oct 23, 2020

mlbridge bot commented Oct 23, 2020 •

edited

Loading

shipilev left a comment

shipilev Oct 23, 2020 •

edited

Loading

tschatzl commented Oct 23, 2020

shipilev left a comment

openjdk bot commented Oct 23, 2020 •

edited

Loading

kimbarrett Oct 26, 2020

tschatzl commented Oct 27, 2020

tschatzl commented Oct 29, 2020

openjdk bot commented Oct 29, 2020

8255298: Remove SurvivorAlignmentInBytes functionality #838

8255298: Remove SurvivorAlignmentInBytes functionality #838

Conversation

tschatzl commented Oct 23, 2020 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Oct 23, 2020

openjdk bot commented Oct 23, 2020

mlbridge bot commented Oct 23, 2020 • edited Loading

Webrevs

shipilev left a comment

Choose a reason for hiding this comment

shipilev Oct 23, 2020 • edited Loading

Choose a reason for hiding this comment

tschatzl commented Oct 23, 2020

shipilev left a comment

Choose a reason for hiding this comment

openjdk bot commented Oct 23, 2020 • edited Loading

kimbarrett Oct 26, 2020

Choose a reason for hiding this comment

tschatzl commented Oct 27, 2020

tschatzl commented Oct 29, 2020

openjdk bot commented Oct 29, 2020

tschatzl commented Oct 23, 2020 •

edited by openjdk bot

Loading

mlbridge bot commented Oct 23, 2020 •

edited

Loading

shipilev Oct 23, 2020 •

edited

Loading

openjdk bot commented Oct 23, 2020 •

edited

Loading