Re: RFR: 8272481: [macos] javax/swing/JFrame/NSTexturedJFrame/NSTexturedJFrame.java fails [v4]
On Wed, 25 Aug 2021 17:32:46 GMT, Alexey Ushakov wrote: >> Update opacity only if the component is visible > > Alexey Ushakov has updated the pull request incrementally with one additional > commit since the last revision: > > 8272481: [macos] javax/swing/JFrame/NSTexturedJFrame/NSTexturedJFrame.java > fails > > Move the initialization to better place Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5172
Re: RFR: 8271603: Unnecessary Vector usage in java.desktop [v4]
> Usage of thread-safe collection `Vector` is unnecessary. It's recommended to > use `ArrayList` if a thread-safe implementation is not needed. In > post-BiasedLocking times, this is gets worse, as every access is synchronized. > I checked only places where `Vector` was used as local variable. Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8271603: Unnecessary Vector usage in java.desktop migrate even more usages - Changes: - all: https://git.openjdk.java.net/jdk/pull/4680/files - new: https://git.openjdk.java.net/jdk/pull/4680/files/97ca8477..5727f6f2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4680&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4680&range=02-03 Stats: 30 lines in 7 files changed: 3 ins; 2 del; 25 mod Patch: https://git.openjdk.java.net/jdk/pull/4680.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4680/head:pull/4680 PR: https://git.openjdk.java.net/jdk/pull/4680
Re: RFR: 8272481: [macos] javax/swing/JFrame/NSTexturedJFrame/NSTexturedJFrame.java fails [v3]
On Tue, 24 Aug 2021 23:56:54 GMT, Sergey Bylokhov wrote:
>> Alexey Ushakov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8272481: [macos] javax/swing/JFrame/NSTexturedJFrame/NSTexturedJFrame.java
>> fails
>>
>> Moved peer.setTextured() to the place where windowLayer is initialized
>
> src/java.desktop/macosx/classes/sun/lwawt/macosx/CPlatformWindow.java line
> 339:
>
>> 337: if (peer != null) { // Not applicable to CWarningWindow
>> 338: peer.setTextured(IS(TEXTURED, styleBits));
>> 339: }
>
> This probably works fine, but maybe we can move it after "setPtr(ref.get());"
> below? or that ptr is not used in this new call chain?
Yes, it does work. But it's a good idea to move the code here.
-
PR: https://git.openjdk.java.net/jdk/pull/5172
Re: RFR: 8272481: [macos] javax/swing/JFrame/NSTexturedJFrame/NSTexturedJFrame.java fails [v4]
> Update opacity only if the component is visible Alexey Ushakov has updated the pull request incrementally with one additional commit since the last revision: 8272481: [macos] javax/swing/JFrame/NSTexturedJFrame/NSTexturedJFrame.java fails Move the initialization to better place - Changes: - all: https://git.openjdk.java.net/jdk/pull/5172/files - new: https://git.openjdk.java.net/jdk/pull/5172/files/3f59951b..e68e2687 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5172&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5172&range=02-03 Stats: 6 lines in 1 file changed: 2 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5172.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5172/head:pull/5172 PR: https://git.openjdk.java.net/jdk/pull/5172
Re: RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules [v3]
> Collections.sort is just a wrapper, so it is better to use an instance method > directly. Andrey Turbanov has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5229/files - new: https://git.openjdk.java.net/jdk/pull/5229/files/d6dfc8bf..e31936a5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5229&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5229&range=01-02 Stats: 21 lines in 10 files changed: 4 ins; 0 del; 17 mod Patch: https://git.openjdk.java.net/jdk/pull/5229.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5229/head:pull/5229 PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules [v2]
On Wed, 25 Aug 2021 12:47:41 GMT, Andrey Turbanov wrote: >> src/java.base/share/classes/java/net/URLPermission.java line 222: >> >>> 220: >>> 221: List l = normalizeMethods(methods); >>> 222: l.sort(null); >> >> I am not opposed to this change, but I find this is slightly more ugly than >> `Collections.sort(l)`; so I have to ask: Is there any noticeable benefit? > > Actually I agree with you. > One more point is that List.sort(null) doesn't check at compile time that > class implements `Comparable`. But Collections.sort have this check. > I will revert last commit. > @azvegint are you ok with that? No objections. - PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules [v2]
On Wed, 25 Aug 2021 08:29:57 GMT, Daniel Fuchs wrote: >> Andrey Turbanov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8272863: Replace usages of Collections.sort with List.sort call in public >> java modules >> replace Collections.sort without comparator too > > src/java.base/share/classes/java/net/URLPermission.java line 222: > >> 220: >> 221: List l = normalizeMethods(methods); >> 222: l.sort(null); > > I am not opposed to this change, but I find this is slightly more ugly than > `Collections.sort(l)`; so I have to ask: Is there any noticeable benefit? Actually I agree with you. One more point is that List.sort(null) doesn't check at compile time that class implements `Comparable`. But Collections.sort have this check. I will revert last commit. @azvegint are you ok with that? - PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules [v2]
On Tue, 24 Aug 2021 20:21:52 GMT, Andrey Turbanov wrote: >> Collections.sort is just a wrapper, so it is better to use an instance >> method directly. > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8272863: Replace usages of Collections.sort with List.sort call in public > java modules > replace Collections.sort without comparator too src/java.base/share/classes/java/net/URLPermission.java line 222: > 220: > 221: List l = normalizeMethods(methods); > 222: l.sort(null); I am not opposed to this change, but I find this is slightly more ugly than `Collections.sort(l)`; so I have to ask: Is there any noticeable benefit? - PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8271603: Unnecessary Vector usage in java.desktop [v2]
On Tue, 24 Aug 2021 23:09:52 GMT, Sergey Bylokhov wrote: >> Andrey Turbanov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8271603: Unnecessary Vector usage in java.desktop >> revert back to Enumeration >> bring back default values > > src/java.desktop/share/classes/javax/sound/sampled/AudioSystem.java line 244: > >> 242: } >> 243: >> 244: return list.toArray(new Line.Info[0]); > > I thought we already covered such changed under JDK-8269130, is there are any > other missed cases? If yes then probably it will be good to extract such > changes. In JDK-8269130 I fixed only usages of Collections.toArray. and didn't touch manual copying. Reverted usage of toArray where it wasn't used before. - PR: https://git.openjdk.java.net/jdk/pull/4680
Re: RFR: 8271603: Unnecessary Vector usage in java.desktop [v3]
> Usage of thread-safe collection `Vector` is unnecessary. It's recommended to > use `ArrayList` if a thread-safe implementation is not needed. In > post-BiasedLocking times, this is gets worse, as every access is synchronized. > I checked only places where `Vector` was used as local variable. Andrey Turbanov has updated the pull request incrementally with two additional commits since the last revision: - 8271603: Unnecessary Vector usage in java.desktop migrate more usages. Not sure how I missed them - 8271603: Unnecessary Vector usage in java.desktop revert back to use cycle to copy into array - Changes: - all: https://git.openjdk.java.net/jdk/pull/4680/files - new: https://git.openjdk.java.net/jdk/pull/4680/files/7a2393eb..97ca8477 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4680&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4680&range=01-02 Stats: 214 lines in 24 files changed: 26 ins; 11 del; 177 mod Patch: https://git.openjdk.java.net/jdk/pull/4680.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4680/head:pull/4680 PR: https://git.openjdk.java.net/jdk/pull/4680
Re: RFR: 8271603: Unnecessary Vector usage in java.desktop [v2]
On Tue, 24 Aug 2021 21:13:57 GMT, Andrey Turbanov wrote: >> Usage of thread-safe collection `Vector` is unnecessary. It's recommended to >> use `ArrayList` if a thread-safe implementation is not needed. In >> post-BiasedLocking times, this is gets worse, as every access is >> synchronized. >> I checked only places where `Vector` was used as local variable. > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8271603: Unnecessary Vector usage in java.desktop > revert back to Enumeration > bring back default values I found many new places in java.desktop which can be migrated easily. Not sure how I did miss them in the first place. Sorry for inconvenience. Please re-review PR. - PR: https://git.openjdk.java.net/jdk/pull/4680
