RFR: 8273541: Cleaner Thread creates with normal priority instead of MAX_PRIORITY - 2
…of MAX_PRIORITY-2 during refactoring Appears in Java 17 for the first time. During refactoring, the priority was changed from Thread.MAX_PRIORITY - 2 to instead state Thread.MIN_PRIORITY - 2, which results in a negative priority, and is thus set to Thread.NORM_PRIORITY. Thus the Cleaner by default now has threads with priority 5, instead of 8. The change was done in git revision # 992b50087d2ec8878dfcbbd1820a00b6b6bdf644 and label 8261036 by Claes Redestad ------------- Commit messages: - Merge branch 'openjdk:master' into thread_priority - Merge remote-tracking branch 'origin/thread_priority' into thread_priority - Merge branch 'openjdk:master' into thread_priority - Merge branch 'openjdk:master' into thread_priority - Merge branch 'openjdk:master' into thread_priority - Redo the change to MAX_PRIORITY - 2 - Removed stray "java" - Thread priority of Cleaner threads was set to MIN_PRIORITY-2 instead of MAX_PRIORITY-2 during refactoring Changes: https://git.openjdk.java.net/jdk/pull/5439/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5439&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273541 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5439.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5439/head:pull/5439 PR: https://git.openjdk.java.net/jdk/pull/5439
On Thu, 9 Sep 2021 10:14:48 GMT, kabutz <github.com+332398+kabutz@openjdk.org> wrote:
…of MAX_PRIORITY-2 during refactoring
Appears in Java 17 for the first time.
During refactoring, the priority was changed from Thread.MAX_PRIORITY - 2 to instead state Thread.MIN_PRIORITY - 2, which results in a negative priority, and is thus set to Thread.NORM_PRIORITY. Thus the Cleaner by default now has threads with priority 5, instead of 8.
The change was done in git revision # 992b50087d2ec8878dfcbbd1820a00b6b6bdf644 and label 8261036 by Claes Redestad
This looks good. @cl4es might want to confirm that change was not intentional. Please change the PR title to "8273541: Cleaner Thread creates with normal priority instead of MAX_PRIORITY - 2" to get it automatically hooked by bots. src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java line 228:
226: super(cleaner, cleaner); 227: } 228: java
Stray change. ------------- Changes requested by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5439
On Thu, 9 Sep 2021 13:38:32 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
…of MAX_PRIORITY-2 during refactoring
Appears in Java 17 for the first time.
During refactoring, the priority was changed from Thread.MAX_PRIORITY - 2 to instead state Thread.MIN_PRIORITY - 2, which results in a negative priority, and is thus set to Thread.NORM_PRIORITY. Thus the Cleaner by default now has threads with priority 5, instead of 8.
The change was done in git revision # 992b50087d2ec8878dfcbbd1820a00b6b6bdf644 and label 8261036 by Claes Redestad
Please change the PR title to "8273541: Cleaner Thread creates with normal priority instead of MAX_PRIORITY - 2" to get it automatically hooked by bots.
Why would jcheck say "This PR contains no changes" @shipilev ? ------------- PR: https://git.openjdk.java.net/jdk/pull/5439
On Thu, 9 Sep 2021 13:37:23 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
…of MAX_PRIORITY-2 during refactoring
Appears in Java 17 for the first time.
During refactoring, the priority was changed from Thread.MAX_PRIORITY - 2 to instead state Thread.MIN_PRIORITY - 2, which results in a negative priority, and is thus set to Thread.NORM_PRIORITY. Thus the Cleaner by default now has threads with priority 5, instead of 8.
The change was done in git revision # 992b50087d2ec8878dfcbbd1820a00b6b6bdf644 and label 8261036 by Claes Redestad
src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java line 228:
226: super(cleaner, cleaner); 227: } 228: java
Stray change.
Not sure what the "java" is for here seems like a typo ------------- PR: https://git.openjdk.java.net/jdk/pull/5439
On Thu, 9 Sep 2021 15:36:08 GMT, Lance Andersen <lancea@openjdk.org> wrote:
src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java line 228:
226: super(cleaner, cleaner); 227: } 228: java
Stray change.
Not sure what the "java" is for here seems like a typo
Yes, sorry. ------------- PR: https://git.openjdk.java.net/jdk/pull/5439
On Thu, 9 Sep 2021 13:37:45 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
This looks good. @cl4es might want to confirm that change was not intentional.
Completely unintentional, and perplexing since it's not a simple copy-paste error.. ------------- PR: https://git.openjdk.java.net/jdk/pull/5439
On Fri, 10 Sep 2021 14:08:05 GMT, Claes Redestad <redestad@openjdk.org> wrote:
This looks good. @cl4es might want to confirm that change was not intentional.
Completely unintentional, and perplexing since it's not a simple copy-paste error..
The thread priorities in Java are the wrong way round, (high is 10, low is 1) so I was not surprised by this mistake. ------------- PR: https://git.openjdk.java.net/jdk/pull/5439
On 11/09/2021 5:24 am, kabutz wrote:
On Fri, 10 Sep 2021 14:08:05 GMT, Claes Redestad <redestad@openjdk.org> wrote:
This looks good. @cl4es might want to confirm that change was not intentional.
Completely unintentional, and perplexing since it's not a simple copy-paste error..
The thread priorities in Java are the wrong way round, (high is 10, low is 1) so I was not surprised by this mistake.
They are not "the wrong way around" they followed Solaris Global Dispatch Priorities (and Windows priorities for that matter). It makes a lot more sense than systems where a "higher priority" means a lower numerical priority value IMO. :) Cheers, David
-------------
On Thu, 9 Sep 2021 10:14:48 GMT, kabutz <github.com+332398+kabutz@openjdk.org> wrote:
…of MAX_PRIORITY-2 during refactoring
Appears in Java 17 for the first time.
During refactoring, the priority was changed from Thread.MAX_PRIORITY - 2 to instead state Thread.MIN_PRIORITY - 2, which results in a negative priority, and is thus set to Thread.NORM_PRIORITY. Thus the Cleaner by default now has threads with priority 5, instead of 8.
The change was done in git revision # 992b50087d2ec8878dfcbbd1820a00b6b6bdf644 and label 8261036 by Claes Redestad
I've created JDK-8273541 to track this. It looks like d621cdd957b78e53ea63b6b9bb2fce08560a8620 has undone the changes. ------------- PR: https://git.openjdk.java.net/jdk/pull/5439
On Thu, 9 Sep 2021 10:14:48 GMT, kabutz <github.com+332398+kabutz@openjdk.org> wrote:
…of MAX_PRIORITY-2 during refactoring
Appears in Java 17 for the first time.
During refactoring, the priority was changed from Thread.MAX_PRIORITY - 2 to instead state Thread.MIN_PRIORITY - 2, which results in a negative priority, and is thus set to Thread.NORM_PRIORITY. Thus the Cleaner by default now has threads with priority 5, instead of 8.
The change was done in git revision # 992b50087d2ec8878dfcbbd1820a00b6b6bdf644 and label 8261036 by Claes Redestad
It looks like [d621cdd](https://github.com/openjdk/jdk/commit/d621cdd957b78e53ea63b6b9bb2fce08560a86...) has undone the changes.
Thanks Alan! ------------- PR: https://git.openjdk.java.net/jdk/pull/5439
On Thu, 9 Sep 2021 10:14:48 GMT, kabutz <github.com+332398+kabutz@openjdk.org> wrote:
…of MAX_PRIORITY-2 during refactoring
Appears in Java 17 for the first time.
During refactoring, the priority was changed from Thread.MAX_PRIORITY - 2 to instead state Thread.MIN_PRIORITY - 2, which results in a negative priority, and is thus set to Thread.NORM_PRIORITY. Thus the Cleaner by default now has threads with priority 5, instead of 8.
The change was done in git revision # 992b50087d2ec8878dfcbbd1820a00b6b6bdf644 and label 8261036 by Claes Redestad
This looks fine to me. ------------- Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5439
On Thu, 9 Sep 2021 10:14:48 GMT, kabutz <github.com+332398+kabutz@openjdk.org> wrote:
…of MAX_PRIORITY-2 during refactoring
Appears in Java 17 for the first time.
During refactoring, the priority was changed from Thread.MAX_PRIORITY - 2 to instead state Thread.MIN_PRIORITY - 2, which results in a negative priority, and is thus set to Thread.NORM_PRIORITY. Thus the Cleaner by default now has threads with priority 5, instead of 8.
The change was done in git revision # 992b50087d2ec8878dfcbbd1820a00b6b6bdf644 and label 8261036 by Claes Redestad
Marked as reviewed by alanb (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/5439
On Thu, 9 Sep 2021 10:14:48 GMT, kabutz <github.com+332398+kabutz@openjdk.org> wrote:
…of MAX_PRIORITY-2 during refactoring
Appears in Java 17 for the first time.
During refactoring, the priority was changed from Thread.MAX_PRIORITY - 2 to instead state Thread.MIN_PRIORITY - 2, which results in a negative priority, and is thus set to Thread.NORM_PRIORITY. Thus the Cleaner by default now has threads with priority 5, instead of 8.
The change was done in git revision # 992b50087d2ec8878dfcbbd1820a00b6b6bdf644 and label 8261036 by Claes Redestad
This should be ready to integrate so please enter /integrate in a new comment and then it can be sponsored. Thank you ------------- Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5439
On Thu, 9 Sep 2021 10:14:48 GMT, kabutz <github.com+332398+kabutz@openjdk.org> wrote:
…of MAX_PRIORITY-2 during refactoring
Appears in Java 17 for the first time.
During refactoring, the priority was changed from Thread.MAX_PRIORITY - 2 to instead state Thread.MIN_PRIORITY - 2, which results in a negative priority, and is thus set to Thread.NORM_PRIORITY. Thus the Cleaner by default now has threads with priority 5, instead of 8.
The change was done in git revision # 992b50087d2ec8878dfcbbd1820a00b6b6bdf644 and label 8261036 by Claes Redestad
This pull request has now been integrated. Changeset: 4e6de5f9 Author: kabutz <heinz@javaspecialists.eu> Committer: Claes Redestad <redestad@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/4e6de5f9defc031158e027bcbbaf6232b8c0... Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8273541: Cleaner Thread creates with normal priority instead of MAX_PRIORITY - 2 Reviewed-by: shade, alanb, lancea ------------- PR: https://git.openjdk.java.net/jdk/pull/5439
participants (6)
-
Alan Bateman
-
Aleksey Shipilev
-
Claes Redestad
-
David Holmes
-
kabutz
-
Lance Andersen