Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Thu, 21 Apr 2022 10:13:11 GMT, Lance Andersen wrote: > Looks fine Brian. Any thoughts as to whether a release note is warranted? Thanks, @LanceAndersen. The issue is labelled as needing a release note so you are spot on. - PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Wed, 20 Apr 2022 15:33:26 GMT, Brian Burkhalter wrote: >> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods >> of `java.io.FilterInputStream`. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8284930: Also remove synchronized keyword from mark() and reset() of > InputStream Looks fine Brian. Any thoughts as to whether a release note is warranted? - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Wed, 20 Apr 2022 15:33:26 GMT, Brian Burkhalter wrote: >> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods >> of `java.io.FilterInputStream`. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8284930: Also remove synchronized keyword from mark() and reset() of > InputStream Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Wed, 20 Apr 2022 15:33:26 GMT, Brian Burkhalter wrote: >> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods >> of `java.io.FilterInputStream`. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8284930: Also remove synchronized keyword from mark() and reset() of > InputStream Marked as reviewed by jpai (Committer). - PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Wed, 20 Apr 2022 15:33:26 GMT, Brian Burkhalter wrote: >> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods >> of `java.io.FilterInputStream`. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8284930: Also remove synchronized keyword from mark() and reset() of > InputStream Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Wed, 20 Apr 2022 15:33:26 GMT, Brian Burkhalter wrote: >> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods >> of `java.io.FilterInputStream`. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8284930: Also remove synchronized keyword from mark() and reset() of > InputStream Oh, sorry, I had missed that synchronization had already been removed from the `InputStream` methods. Needless to say, I approve. :-) - PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Thu, 21 Apr 2022 00:00:47 GMT, Stuart Marks wrote: > > I think it's a vanishingly small possibility that anything is relying on the > synchronization in these methods. Synchronization here would provide proper > memory visibility effects across threads. To use input streams from multiple > threads without interleaving operations, one would have to provide some other > means of coordination among those threads, which itself would likely ensure > proper memory visibility. I'm hard pressed to see how threads could > coordinate solely via use of the `mark` and `reset` methods. Therefore, I > think it's safe to remove synchronization from them. > > This reasoning also applies to `InputStream`. Perhaps synchronization can be > removed from there too. > > I agree that a CSR is probably warranted to capture the behavior change and > analysis. Commit f210cbf5f6bf58326a379b4b3f55fddf49d30f5c removed the synchronization from `InputStream`'s `mark()` and `reset()`; a CSR is on file. - PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Wed, 20 Apr 2022 15:33:26 GMT, Brian Burkhalter wrote: >> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods >> of `java.io.FilterInputStream`. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8284930: Also remove synchronized keyword from mark() and reset() of > InputStream I think it's a vanishingly small possibility that anything is relying on the synchronization in these methods. Synchronization here would provide proper memory visibility effects across threads. To use input streams from multiple threads without interleaving operations, one would have to provide some other means of coordination among those threads, which itself would likely ensure proper memory visibility. I'm hard pressed to see how threads could coordinate solely via use of the `mark` and `reset` methods. Therefore, I think it's safe to remove synchronization from them. This reasoning also applies to `InputStream`. Perhaps synchronization can be removed from there too. I agree that a CSR is probably warranted to capture the behavior change and analysis. - PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Wed, 20 Apr 2022 18:40:13 GMT, Alan Bateman wrote: > Thanks for the update. I can't think how anything could depend on this but > unfortunately it's been like that since early JDK releases. We will need a > release note, I'm sure Joe will give his opinion on whether a CSR should be > created. Hmm. It is true that the presence or absence of the "synchronized" modifier is not part of a method's contract. However, the multi-thread safety, or not, of a class is part of its behavioral contract. Without a lot of investigation, my current inclination is to run a CSR for this change for the (small) behavioral compatibility hazard. - PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Wed, 20 Apr 2022 15:33:26 GMT, Brian Burkhalter wrote: >> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods >> of `java.io.FilterInputStream`. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8284930: Also remove synchronized keyword from mark() and reset() of > InputStream Thanks for the update. I can't think how anything could depend on this but unfortunately it's been like that since early JDK releases. We will need a release note, I'm sure Joe will give his opinion on whether a CSR should be created. - PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods > of `java.io.FilterInputStream`. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8284930: Also remove synchronized keyword from mark() and reset() of InputStream - Changes: - all: https://git.openjdk.java.net/jdk/pull/8309/files - new: https://git.openjdk.java.net/jdk/pull/8309/files/85546c9e..f210cbf5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8309&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8309&range=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8309.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8309/head:pull/8309 PR: https://git.openjdk.java.net/jdk/pull/8309
