Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
> This update addresses performance issues across both LinkedTransferQueue and > SynchronousQueue by creating a common basis for implementation across them > (mainly in LinkedTransferQueue). Pasting from internal doc summary of > changes: > * * Class DualNode replaces Qnode, with fields and methods > * that apply to any match-based dual data structure, and now > * usable in other j.u.c classes. in particular, SynchronousQueue. > * * Blocking control (in class DualNode) accommodates > * VirtualThreads and (perhaps virtualized) uniprocessors. > * * All fields of this class (LinkedTransferQueue) are > * default-initializable (to null), allowing further extension > * (in particular, SynchronousQueue.Transferer) > * * Head and tail fields are lazily initialized rather than set > * to a dummy node, while also reducing retries under heavy > * contention and misorderings, and relaxing some accesses, > * requiring accommodation in many places (as well as > * adjustments in WhiteBox tests). Doug Lea has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision: - Merge branch 'openjdk:master' into JDK-8301341 - Address more review comments - Address review comments - nitpicks - Merge branch 'openjdk:master' into JDK-8301341 - Accommodate white-box tests; use consistent constructions; minor improvements - Merge branch 'openjdk:master' into JDK-8301341 - Simplify contention handling; fix test - Fix inverted test assert; improve internal documentation; simplify code - Merge branch 'openjdk:master' into JDK-8301341 - ... and 3 more: https://git.openjdk.org/jdk/compare/3e585ff1...f53cee67 - Changes: - all: https://git.openjdk.org/jdk/pull/14317/files - new: https://git.openjdk.org/jdk/pull/14317/files/7c64a933..f53cee67 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14317&range=09 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14317&range=08-09 Stats: 5915 lines in 182 files changed: 4676 ins; 702 del; 537 mod Patch: https://git.openjdk.org/jdk/pull/14317.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14317/head:pull/14317 PR: https://git.openjdk.org/jdk/pull/14317
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Fri, 21 Jul 2023 16:29:24 GMT, Doug Lea wrote:
>> src/java.base/share/classes/java/util/concurrent/LinkedTransferQueue.java
>> line 896:
>>
>>> 894: for (DualNode p = (pred == null) ? head : pred.next, c = p;
>>> 895: p != null; ) {
>>> 896: boolean isData = p.isData;
>>
>> @DougLea Are you finding that manual hoisting of reads to final members has
>> a perf edge? 🤔
>
> Collection-support code from method countOfMode on down was minimally touched
> from its previous version. This shows up as a style clash into very branchy
> self-link-coping code. Changing might require more changes to WhiteBox tests
> that are finicky about node counts and self-links expected in traversals etc.
> (These tests are useful but are themselves hard to change to accommodate
> small implementation differences.)
@DougLea Alright, good to know :)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14317#discussion_r1271061647
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Fri, 21 Jul 2023 16:42:25 GMT, Doug Lea wrote: >> This update addresses performance issues across both LinkedTransferQueue and >> SynchronousQueue by creating a common basis for implementation across them >> (mainly in LinkedTransferQueue). Pasting from internal doc summary of >> changes: >> * * Class DualNode replaces Qnode, with fields and methods >> * that apply to any match-based dual data structure, and now >> * usable in other j.u.c classes. in particular, SynchronousQueue. >> * * Blocking control (in class DualNode) accommodates >> * VirtualThreads and (perhaps virtualized) uniprocessors. >> * * All fields of this class (LinkedTransferQueue) are >> * default-initializable (to null), allowing further extension >> * (in particular, SynchronousQueue.Transferer) >> * * Head and tail fields are lazily initialized rather than set >> * to a dummy node, while also reducing retries under heavy >> * contention and misorderings, and relaxing some accesses, >> * requiring accommodation in many places (as well as >> * adjustments in WhiteBox tests). > > Doug Lea has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 13 additional commits since > the last revision: > > - Merge branch 'openjdk:master' into JDK-8301341 > - Address more review comments > - Address review comments > - nitpicks > - Merge branch 'openjdk:master' into JDK-8301341 > - Accommodate white-box tests; use consistent constructions; minor > improvements > - Merge branch 'openjdk:master' into JDK-8301341 > - Simplify contention handling; fix test > - Fix inverted test assert; improve internal documentation; simplify code > - Merge branch 'openjdk:master' into JDK-8301341 > - ... and 3 more: https://git.openjdk.org/jdk/compare/c456c802...f53cee67 Thanks for making the fixes Doug! Would it also be possible to backport these fixes to Java 17? It seems to be a very common issue for openHAB users now that they upgrade to openHAB 4 which requires Java 17. See: https://community.openhab.org/t/consistent-100-cpu-use-of-safecall-queue-thread/143792 - PR Comment: https://git.openjdk.org/jdk/pull/14317#issuecomment-1666841611
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Sun, 6 Aug 2023 12:23:54 GMT, Wouter Born wrote: >> Doug Lea has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes the unrelated changes brought >> in by the merge/rebase. The pull request contains 13 additional commits >> since the last revision: >> >> - Merge branch 'openjdk:master' into JDK-8301341 >> - Address more review comments >> - Address review comments >> - nitpicks >> - Merge branch 'openjdk:master' into JDK-8301341 >> - Accommodate white-box tests; use consistent constructions; minor >> improvements >> - Merge branch 'openjdk:master' into JDK-8301341 >> - Simplify contention handling; fix test >> - Fix inverted test assert; improve internal documentation; simplify code >> - Merge branch 'openjdk:master' into JDK-8301341 >> - ... and 3 more: https://git.openjdk.org/jdk/compare/c067d356...f53cee67 > > Thanks for making the fixes Doug! > Would it also be possible to backport these fixes to Java 17? > > It seems to be a very common issue for openHAB users now that they upgrade to > openHAB 4 which requires Java 17. > > See: > > * > https://community.openhab.org/t/consistent-100-cpu-use-of-safecall-queue-thread/143792 > * https://community.openhab.org/t/high-cpu-usage-after-migration-to-oh4/148200 @wborn I think 17 should also be OK modulo deleting 2 lines for pre-21 mentioned above. I only checked with 19 though.. - PR Comment: https://git.openjdk.org/jdk/pull/14317#issuecomment-1673399971
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Thu, 10 Aug 2023 15:03:18 GMT, Doug Lea wrote: >> Thanks for making the fixes Doug! >> Would it also be possible to backport these fixes to Java 17? >> >> It seems to be a very common issue for openHAB users now that they upgrade >> to openHAB 4 which requires Java 17. >> >> See: >> >> * >> https://community.openhab.org/t/consistent-100-cpu-use-of-safecall-queue-thread/143792 >> * >> https://community.openhab.org/t/high-cpu-usage-after-migration-to-oh4/148200 > > @wborn I think 17 should also be OK modulo deleting 2 lines for pre-21 > mentioned above. I only checked with 19 though.. @DougLea is there any timeline where we can expect the backport of this fix into jdk17? or any other work around? - PR Comment: https://git.openjdk.org/jdk/pull/14317#issuecomment-2203971692
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Tue, 2 Jul 2024 17:55:31 GMT, Suryanarayana Garlapati wrote: >> @wborn I think 17 should also be OK modulo deleting 2 lines for pre-21 >> mentioned above. I only checked with 19 though.. > > @DougLea is there any timeline where we can expect the backport of this fix > into jdk17? or any other work around? @suryag10 Sorry I'm not the right person to ask about backports. - PR Comment: https://git.openjdk.org/jdk/pull/14317#issuecomment-2204174455
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Fri, 21 Jul 2023 16:42:25 GMT, Doug Lea wrote: >> This update addresses performance issues across both LinkedTransferQueue and >> SynchronousQueue by creating a common basis for implementation across them >> (mainly in LinkedTransferQueue). Pasting from internal doc summary of >> changes: >> * * Class DualNode replaces Qnode, with fields and methods >> * that apply to any match-based dual data structure, and now >> * usable in other j.u.c classes. in particular, SynchronousQueue. >> * * Blocking control (in class DualNode) accommodates >> * VirtualThreads and (perhaps virtualized) uniprocessors. >> * * All fields of this class (LinkedTransferQueue) are >> * default-initializable (to null), allowing further extension >> * (in particular, SynchronousQueue.Transferer) >> * * Head and tail fields are lazily initialized rather than set >> * to a dummy node, while also reducing retries under heavy >> * contention and misorderings, and relaxing some accesses, >> * requiring accommodation in many places (as well as >> * adjustments in WhiteBox tests). > > Doug Lea has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 13 additional commits since > the last revision: > > - Merge branch 'openjdk:master' into JDK-8301341 > - Address more review comments > - Address review comments > - nitpicks > - Merge branch 'openjdk:master' into JDK-8301341 > - Accommodate white-box tests; use consistent constructions; minor > improvements > - Merge branch 'openjdk:master' into JDK-8301341 > - Simplify contention handling; fix test > - Fix inverted test assert; improve internal documentation; simplify code > - Merge branch 'openjdk:master' into JDK-8301341 > - ... and 3 more: https://git.openjdk.org/jdk/compare/37ac993c...f53cee67 Like Doug notes, the JDK backports are managed as a separate project. Details are available here https://openjdk.org/projects/jdk-updates/. For JDK 17 the corresponding project is https://wiki.openjdk.org/display/JDKUpdates/JDK+17u which has the necessary details about the process as well as the mailing list details where you can bring up the backporting question. - PR Comment: https://git.openjdk.org/jdk/pull/14317#issuecomment-2204851286
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Tue, 2 Jul 2024 19:28:58 GMT, Doug Lea wrote: >> @DougLea is there any timeline where we can expect the backport of this fix >> into jdk17? or any other work around? > > @suryag10 Sorry I'm not the right person to ask about backports. Thanks for the info @DougLea and @jaikiran - PR Comment: https://git.openjdk.org/jdk/pull/14317#issuecomment-2205061210
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Fri, 21 Jul 2023 16:42:25 GMT, Doug Lea wrote: >> This update addresses performance issues across both LinkedTransferQueue and >> SynchronousQueue by creating a common basis for implementation across them >> (mainly in LinkedTransferQueue). Pasting from internal doc summary of >> changes: >> * * Class DualNode replaces Qnode, with fields and methods >> * that apply to any match-based dual data structure, and now >> * usable in other j.u.c classes. in particular, SynchronousQueue. >> * * Blocking control (in class DualNode) accommodates >> * VirtualThreads and (perhaps virtualized) uniprocessors. >> * * All fields of this class (LinkedTransferQueue) are >> * default-initializable (to null), allowing further extension >> * (in particular, SynchronousQueue.Transferer) >> * * Head and tail fields are lazily initialized rather than set >> * to a dummy node, while also reducing retries under heavy >> * contention and misorderings, and relaxing some accesses, >> * requiring accommodation in many places (as well as >> * adjustments in WhiteBox tests). > > Doug Lea has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 13 additional commits since > the last revision: > > - Merge branch 'openjdk:master' into JDK-8301341 > - Address more review comments > - Address review comments > - nitpicks > - Merge branch 'openjdk:master' into JDK-8301341 > - Accommodate white-box tests; use consistent constructions; minor > improvements > - Merge branch 'openjdk:master' into JDK-8301341 > - Simplify contention handling; fix test > - Fix inverted test assert; improve internal documentation; simplify code > - Merge branch 'openjdk:master' into JDK-8301341 > - ... and 3 more: https://git.openjdk.org/jdk/compare/633a84d0...f53cee67 Just my 2 cents—given the size of this change, I'd be hesitant to have it backported. Also, more than this PR would need to be backported, for instance: https://github.com/openjdk/jdk/pull/19271 - PR Comment: https://git.openjdk.org/jdk/pull/14317#issuecomment-2205567436
