Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v3]
On Fri, 11 Dec 2020 15:07:45 GMT, Petr Janeček wrote: >> The changes to doc comments look good. >> >> Thanks for incorporating my earlier code snippet suggestions published on >> [concurrency-interest](http://cs.oswego.edu/pipermail/concurrency-interest/2020-November/017264.html)! > > This also fixes issue > [JDK-8257671](https://bugs.openjdk.java.net/browse/JDK-8257671): > _ThreadPoolExecutor.Discard*Policy: rejected tasks are not cancelled_, by > updating the `TPE.Discard*Policy` JavaDocs. > > I filed it days before the code went in as I did not see the JavaDoc update > in JDK, sorry for my impatience and thank you for the fix. I cannot update > the Jira, please link it to this PR, too. @JanecekPetr Thank you very much. I've updated JIRA and this pull request for JDK-8257671 - PR: https://git.openjdk.java.net/jdk/pull/1647
Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v3]
On Wed, 9 Dec 2020 18:41:02 GMT, Pavel Rappo wrote: >> Martin Buchholz has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains one commit: >> >> JDK-8234131 > > The changes to doc comments look good. > > Thanks for incorporating my earlier code snippet suggestions published on > [concurrency-interest](http://cs.oswego.edu/pipermail/concurrency-interest/2020-November/017264.html)! This also fixes issue [JDK-8257671](https://bugs.openjdk.java.net/browse/JDK-8257671): _ThreadPoolExecutor.Discard*Policy: rejected tasks are not cancelled_, by updating the `TPE.Discard*Policy` JavaDocs. I filed it days before the code went in as I did not see the JavaDoc update in JDK, sorry for my impatience and thank you for the fix. I cannot update the Jira, please link it to this PR, too. - PR: https://git.openjdk.java.net/jdk/pull/1647
Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v3]
Thanks, Pavel!
I committed this to CVS, but am not planning to include it in this
integration:
(not sure if re-generating will invalidate my "Ready" status)
(Having to cast to Future makes this more problematic, but in typical
usages the user will know that the queue contains Futures)
--- src/main/java/util/concurrent/ThreadPoolExecutor.java 8 Dec 2020
20:33:21 - 1.195
+++ src/main/java/util/concurrent/ThreadPoolExecutor.java 9 Dec 2020
22:05:21 - 1.197
@@ -2066,10 +2066,12 @@
* {@code
* new RejectedExecutionHandler() {
* public void rejectedExecution(Runnable r, ThreadPoolExecutor e) {
- * Future dropped = e.getQueue().poll();
- * if (dropped != null)
- *dropped.cancel(false); // also consider logging the failure
- * e.execute(r); // retry
+ * Runnable dropped = e.getQueue().poll();
+ * if (dropped instanceof Future) {
+ * ((Future)dropped).cancel(false);
+ * // also consider logging the failure
+ * }
+ * e.execute(r); // retry
* }}}
*/
public static class DiscardOldestPolicy implements
RejectedExecutionHandler {
On Wed, Dec 9, 2020 at 10:46 AM Pavel Rappo wrote:
> On Tue, 8 Dec 2020 21:15:48 GMT, Martin Buchholz
> wrote:
>
> >> 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12
> >
> > Martin Buchholz has updated the pull request with a new target base due
> to a merge or a rebase. The pull request now contains one commit:
> >
> > JDK-8234131
>
> The changes to doc comments look good.
>
> Thanks for incorporating my earlier code snippet suggestions published on
> [concurrency-interest](
> http://cs.oswego.edu/pipermail/concurrency-interest/2020-November/017264.html
> )!
>
> src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java
> line 2102:
>
> > 2100: *dropped.cancel(false); // also consider logging the
> failure
> > 2101: * e.execute(r); // retry
> > 2102: * }}}
>
> I tried to reify that code snippet:
>
> $ cat MySnippet.java
> import java.util.concurrent.Future;
> import java.util.concurrent.RejectedExecutionHandler;
> import java.util.concurrent.ThreadPoolExecutor;
>
> public class MySnippet {
> public static void main(String[] args) {
> new RejectedExecutionHandler() {
> public void rejectedExecution(Runnable r, ThreadPoolExecutor
> e) {
> Future dropped = e.getQueue().poll();
> if (dropped != null)
> dropped.cancel(false); // also consider logging the
> failure
> e.execute(r); // retry
> }
> };
> }
> }
> $ javac MySnippet.java
> MySnippet.java:9: error: incompatible types: Runnable cannot be converted
> to Future
> Future dropped = e.getQueue().poll();
> ^
> 1 error
> $
> Separately, it seems that the `if` statement uses a 3-space indent which
> is surprising to see in the JSR 166 code base.
>
> -
>
> Marked as reviewed by prappo (Reviewer).
>
> PR: https://git.openjdk.java.net/jdk/pull/1647
>
Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v3]
On Tue, 8 Dec 2020 21:15:48 GMT, Martin Buchholz wrote:
>> 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12
>
> Martin Buchholz has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains one commit:
>
> JDK-8234131
The changes to doc comments look good.
Thanks for incorporating my earlier code snippet suggestions published on
[concurrency-interest](http://cs.oswego.edu/pipermail/concurrency-interest/2020-November/017264.html)!
src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java line
2102:
> 2100: *dropped.cancel(false); // also consider logging the
> failure
> 2101: * e.execute(r); // retry
> 2102: * }}}
I tried to reify that code snippet:
$ cat MySnippet.java
import java.util.concurrent.Future;
import java.util.concurrent.RejectedExecutionHandler;
import java.util.concurrent.ThreadPoolExecutor;
public class MySnippet {
public static void main(String[] args) {
new RejectedExecutionHandler() {
public void rejectedExecution(Runnable r, ThreadPoolExecutor e) {
Future dropped = e.getQueue().poll();
if (dropped != null)
dropped.cancel(false); // also consider logging the failure
e.execute(r); // retry
}
};
}
}
$ javac MySnippet.java
MySnippet.java:9: error: incompatible types: Runnable cannot be converted to
Future
Future dropped = e.getQueue().poll();
^
1 error
$
Separately, it seems that the `if` statement uses a 3-space indent which is
surprising to see in the JSR 166 code base.
-
Marked as reviewed by prappo (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/1647
Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v3]
On Tue, 8 Dec 2020 21:15:48 GMT, Martin Buchholz wrote: >> 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 > > Martin Buchholz has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains one commit: > > JDK-8234131 Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1647
Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v3]
> 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 Martin Buchholz has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit: JDK-8234131 - Changes: https://git.openjdk.java.net/jdk/pull/1647/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1647=02 Stats: 312 lines in 41 files changed: 105 ins; 41 del; 166 mod Patch: https://git.openjdk.java.net/jdk/pull/1647.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1647/head:pull/1647 PR: https://git.openjdk.java.net/jdk/pull/1647
