[OpenJDK 2D-Dev] Integrated: 6986863: ProfileDeferralMgr throwing ConcurrentModificationException
On Thu, 3 Dec 2020 23:52:25 GMT, Sergey Bylokhov wrote: > This change intended to enhance the lazy initialization of the standard color > profiles concurrently by different threads. > > We defer standard profiles loading because most of UI application uses a > small amount of data from the profile like numComponents and colorSpaceType, > and this data is known in advance. But any other profile-related activity > (like a color conversion, profile data accesses, etc.) triggers profile > activation when we load all profile data to the memory. > > Before the fix for JDK-6793818, we defer only one sRGB color profile, see: > https://github.com/openjdk/jdk/commit/2726f2a3621dd2562d4fb660b4c3d376c65027aa > > Notes about the link above: > - The code in the ProfileDeferralMgr, which contain the Vector of profiles > for activation does not use any synchronization > - The `activateDeferredProfile` and `activate` methods are implemented to > throw `ProfileDataException`, but this exception is ignored during activation > process: > > https://github.com/openjdk/jdk/commit/2726f2a3621dd2562d4fb660b4c3d376c65027aa#diff-0839c25a6c999452be28b431c54d5daa91364d302cfda6efa5c56421c2f2bdcbR96 > > The fix: > - Drops the usage of ProfileDeferralMgr (which contained the Vector of > profiles for activation) and ProfileActivator machinery. Instead, we will > have just one `ICC_Profile.activate()` method which will activate and > initialize the `ICC_Profile.cmmProfile` if it is null > - The `activate` method implementation mimics the old behavior when the > CMMException and IOException were wrapped by the ProfileDataException, and > the ProfileDataException itself was ignored during activation - > so an > exception will not be thrown in the method itself, but only when the null > profile will be used. > > See some comments inline. This pull request has now been integrated. Changeset: 64a150c5 Author:Sergey Bylokhov URL: https://git.openjdk.java.net/jdk/commit/64a150c5 Stats: 520 lines in 9 files changed: 183 ins; 265 del; 72 mod 6986863: ProfileDeferralMgr throwing ConcurrentModificationException Reviewed-by: kizune - PR: https://git.openjdk.java.net/jdk/pull/1613
Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v3]
On Thu, 28 Jan 2021 19:50:56 GMT, Sergey Bylokhov wrote: >>> >>> My point is that this is not a test bug, so the test should not be changed. >> >> The test never dispose of the frame. Why is it expected to shut down the >> toolkit? Shall the frame be disposed of when the main thread in the test >> finishes? > >> The test never dispose of the frame. Why is it expected to shut down the >> toolkit? Shall the frame be disposed of when the main thread in the test >> finishes? > > The shutdown is caused by the System.exit call while the toolkit active, so > we should shut down it before the end. It seems "m_breakMessageLoop" is never true for unsuccessful run even though AwtToolkit::QuitMessageLoop finish execution (where m_breakMessageLoop is set to true), so AwtToolkit::MessageLoop never ends, seems to be some timing issue. - PR: https://git.openjdk.java.net/jdk/pull/2220
[OpenJDK 2D-Dev] RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module
This completes the desktop module JNF removal * remove -framework JavaNativeFoundation from make files * remove #import from all source files. If needed add import of JNIUtilities.h to get jni.h definitions - better anyway since then it gets the current JDK ones not the ones from the O/S * replace JNFNSToJavaString with NSStringToJavaString and JNFJavaToNSString with JavaStringToNSString * replace JNFNormalizedNSStringForPath with NormalizedPathNSStringFromJavaString and JNFNormalizedJavaStringForPath with NormalizedPathJavaStringFromNSString * replace JNFGet/ReleaseStringUTF16UniChars with direct calls to JNI * Map all JNFRunLoop perform* calls to the ThreadUtilities versions (the vast majority already did this) * Redo the ThreadUtilities calls to JNFRunLoop to directly invoke NSObject perform* methods. * define new javaRunLoopMode in ThreadUtilities to replace the JNF one and use where needed. * Remove the single usage of JNFPerformEnvBlock * replace JNFJavaToNSNumber in single A11Y file with local replacement * replace single usage of JNFNSTimeIntervalToJavaMillis in ScreenMenu.m with local replacement * remove un-needed JNFRunLoopDidStartNotification from NSApplicationAWT.m * misc. remaining cleanup (eg missed JNF_CHECK_AND_RETHROW_EXCEPTION) - Commit messages: - 8260616: Removing remaining JNF dependencies in the java.desktop module Changes: https://git.openjdk.java.net/jdk/pull/2305/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2305=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8260616 Stats: 431 lines in 71 files changed: 196 ins; 96 del; 139 mod Patch: https://git.openjdk.java.net/jdk/pull/2305.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2305/head:pull/2305 PR: https://git.openjdk.java.net/jdk/pull/2305
Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v3]
On Thu, 28 Jan 2021 12:02:54 GMT, Alexey Ivanov wrote: > The test never dispose of the frame. Why is it expected to shut down the > toolkit? Shall the frame be disposed of when the main thread in the test > finishes? The shutdown is caused by the System.exit call while the toolkit active, so we should shut down it before the end. - PR: https://git.openjdk.java.net/jdk/pull/2220
Re: [OpenJDK 2D-Dev] RFR: JDK-8260518: Change default -mmacosx-version-min to 10.12 [v2]
On Thu, 28 Jan 2021 16:08:53 GMT, Erik Joelsson wrote: >> To guarantee backwards compatible binaries on Macos, we use the option >> -mmacosx-version-min. This is currently set to 10.9, which is a really >> ancient version. I propose we bump this to 10.12, which is still a rather >> conservative old version (support ended in 2019). >> >> The driving issue for bumping this now is the aarch64 port, where building >> for aarch64 requires the version min to be set to 11.0. Having a large gap >> between the target versions becomes problematic as we hit a lot of >> deprecation warnings in shared code. To be able to fix these deprecation >> warnings, we need a smaller version gap. >> >> Just bumping us to 10.12 triggers warnings in libsplashscreen, so I will >> temporarily add "deprecated-declarations" to the list of disabled warnings >> there until they can be fixed in JDK-8260402. > > Erik Joelsson has updated the pull request incrementally with one additional > commit since the last revision: > > Copyright year Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2268
Re: [OpenJDK 2D-Dev] RFR: JDK-8260518: Change default -mmacosx-version-min to 10.12 [v2]
> To guarantee backwards compatible binaries on Macos, we use the option > -mmacosx-version-min. This is currently set to 10.9, which is a really > ancient version. I propose we bump this to 10.12, which is still a rather > conservative old version (support ended in 2019). > > The driving issue for bumping this now is the aarch64 port, where building > for aarch64 requires the version min to be set to 11.0. Having a large gap > between the target versions becomes problematic as we hit a lot of > deprecation warnings in shared code. To be able to fix these deprecation > warnings, we need a smaller version gap. > > Just bumping us to 10.12 triggers warnings in libsplashscreen, so I will > temporarily add "deprecated-declarations" to the list of disabled warnings > there until they can be fixed in JDK-8260402. Erik Joelsson has updated the pull request incrementally with one additional commit since the last revision: Copyright year - Changes: - all: https://git.openjdk.java.net/jdk/pull/2268/files - new: https://git.openjdk.java.net/jdk/pull/2268/files/41d60207..4eb09479 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2268=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2268=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2268.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2268/head:pull/2268 PR: https://git.openjdk.java.net/jdk/pull/2268
[OpenJDK 2D-Dev] Integrated: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory
On Wed, 27 Jan 2021 08:39:33 GMT, Matthias Baesken wrote: > The function AllocateSpaceForGP in freetypeScaler.c calls potentially 2 > times malloc ; however the memory is not always freed correctly in case of > errors. > See also the related sonar issue : > https://sonarcloud.io/project/issues?id=shipilev_jdk=c=AXck8B_SBBG2CXpcngxr=false=BLOCKER=BUG This pull request has now been integrated. Changeset: 3aabbd72 Author:Matthias Baesken URL: https://git.openjdk.java.net/jdk/commit/3aabbd72 Stats: 12 lines in 1 file changed: 8 ins; 0 del; 4 mod 8260432: allocateSpaceForGP in freetypeScaler.c might leak memory Reviewed-by: shade, stuefe - PR: https://git.openjdk.java.net/jdk/pull/2250
Re: [OpenJDK 2D-Dev] RFR: JDK-8260518: Change default -mmacosx-version-min to 10.12
On Wed, 27 Jan 2021 19:23:48 GMT, Erik Joelsson wrote: > To guarantee backwards compatible binaries on Macos, we use the option > -mmacosx-version-min. This is currently set to 10.9, which is a really > ancient version. I propose we bump this to 10.12, which is still a rather > conservative old version (support ended in 2019). > > The driving issue for bumping this now is the aarch64 port, where building > for aarch64 requires the version min to be set to 11.0. Having a large gap > between the target versions becomes problematic as we hit a lot of > deprecation warnings in shared code. To be able to fix these deprecation > warnings, we need a smaller version gap. > > Just bumping us to 10.12 triggers warnings in libsplashscreen, so I will > temporarily add "deprecated-declarations" to the list of disabled warnings > there until they can be fixed in JDK-8260402. Looks good to me. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2268
Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v3]
On Thu, 28 Jan 2021 11:57:06 GMT, Alexey Ivanov wrote: >> It seems in successful run, when the test finish >> - AwtToolkit::MessageLoop starts >> - DoQuitMessageLoop is called >> - AwtToolkit::QuitMessageLoop starts >> - AwtToolkit::QuitMessageLoop finishes >> - AwtToolkit::MessageLoop finish >> - Dispose() is called, m_isDisposed sets to true >> - shutdown hook isDisposed is true so no infinite loop >> >> During unsuccessful run, >> - AwtToolkit::MessageLoop starts >> - DoQuitMessageLoop is called >> - AwtToolkit::QuitMessageLoop starts >> - AwtToolkit::QuitMessageLoop finishes >> - AwtToolkit::MessageLoop NEVER ends so Dispose() is not called so >> m_isDisposed is not set to true so shutdown hook goes in infinite loop. > > I was looking at the code yesterday. Could it be because of synchronisation? > I mean, do we need to use native synchronisation to guarantee variable > changes are seen across the threads? > > Does MessageLoop not receive Quit / Null message? > > My point is that this is not a test bug, so the test should not be changed. The test never dispose of the frame. Why is it expected to shut down the toolkit? Shall the frame be disposed of when the main thread in the test finishes? - PR: https://git.openjdk.java.net/jdk/pull/2220
Re: [OpenJDK 2D-Dev] RFR: 6986863: ProfileDeferralMgr throwing ConcurrentModificationException [v6]
On Thu, 28 Jan 2021 03:44:00 GMT, Sergey Bylokhov wrote: >> This change intended to enhance the lazy initialization of the standard >> color profiles concurrently by different threads. >> >> We defer standard profiles loading because most of UI application uses a >> small amount of data from the profile like numComponents and colorSpaceType, >> and this data is known in advance. But any other profile-related activity >> (like a color conversion, profile data accesses, etc.) triggers profile >> activation when we load all profile data to the memory. >> >> Before the fix for JDK-6793818, we defer only one sRGB color profile, see: >> https://github.com/openjdk/jdk/commit/2726f2a3621dd2562d4fb660b4c3d376c65027aa >> >> Notes about the link above: >> - The code in the ProfileDeferralMgr, which contain the Vector of profiles >> for activation does not use any synchronization >> - The `activateDeferredProfile` and `activate` methods are implemented to >> throw `ProfileDataException`, but this exception is ignored during >> activation process: >> >> https://github.com/openjdk/jdk/commit/2726f2a3621dd2562d4fb660b4c3d376c65027aa#diff-0839c25a6c999452be28b431c54d5daa91364d302cfda6efa5c56421c2f2bdcbR96 >> >> The fix: >> - Drops the usage of ProfileDeferralMgr (which contained the Vector of >> profiles for activation) and ProfileActivator machinery. Instead, we will >> have just one `ICC_Profile.activate()` method which will activate and >> initialize the `ICC_Profile.cmmProfile` if it is null >> - The `activate` method implementation mimics the old behavior when the >> CMMException and IOException were wrapped by the ProfileDataException, and >> the ProfileDataException itself was ignored during activation - > so an >> exception will not be thrown in the method itself, but only when the null >> profile will be used. >> >> See some comments inline. > > Sergey Bylokhov has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 17 commits: > > - Merge branch 'JDK-6986863' of https://github.com/mrserb/jdk into > JDK-6986863 > - Merge branch 'master' into JDK-6986863 > - Merge branch 'master' into JDK-6986863 > - Merge branch 'master' into JDK-6986863 > - update dates > - Update ProfileActivationDuringPropertyAccess.java > - Merge branch 'master' into JDK-6986863 > - Merge branch 'master' into JDK-6986863 > - Merge branch 'master' into JDK-6986863 > - Merge branch 'master' into JDK-6986863 > - ... and 7 more: > https://git.openjdk.java.net/jdk/compare/7030d2e0...e0e59caf Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1613
Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v3]
On Thu, 28 Jan 2021 09:59:22 GMT, Prasanta Sadhukhan wrote: >> Please take a look at the "AwtToolkit::Dispose()" method, on how much stuff >> should be done to properly shutdown the toolkit. This Dispose() method is >> executed immediately when we exit the message loop in the >> "Java_sun_awt_windows_WToolkit_eventLoop". So when the shutdown hook is >> executed we should have the message loop, then we call tk.QuitMessageLoop to >> stop it, and wait until all code in the Dispose() is executed. But since the >> IsDisposed() return false we for unknow reason hang, does it mean that the >> message loop still operates? Or we got some error during "QuitMessageLoop"? > > It seems in successful run, when the test finish > - AwtToolkit::MessageLoop starts > - DoQuitMessageLoop is called > - AwtToolkit::QuitMessageLoop starts > - AwtToolkit::QuitMessageLoop finishes > - AwtToolkit::MessageLoop finish > - Dispose() is called, m_isDisposed sets to true > - shutdown hook isDisposed is true so no infinite loop > > During unsuccessful run, > - AwtToolkit::MessageLoop starts > - DoQuitMessageLoop is called > - AwtToolkit::QuitMessageLoop starts > - AwtToolkit::QuitMessageLoop finishes > - AwtToolkit::MessageLoop NEVER ends so Dispose() is not called so > m_isDisposed is not set to true so shutdown hook goes in infinite loop. I was looking at the code yesterday. Could it be because of synchronisation? I mean, do we need to use native synchronisation to guarantee variable changes are seen across the threads? Does MessageLoop not receive Quit / Null message? - PR: https://git.openjdk.java.net/jdk/pull/2220
Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v3]
On Thu, 28 Jan 2021 05:53:05 GMT, Sergey Bylokhov wrote: >> My point is that this is not a test bug, so the test should not be changed. > > Please take a look at the "AwtToolkit::Dispose()" method, on how much stuff > should be done to properly shutdown the toolkit. This Dispose() method is > executed immediately when we exit the message loop in the > "Java_sun_awt_windows_WToolkit_eventLoop". So when the shutdown hook is > executed we should have the message loop, then we call tk.QuitMessageLoop to > stop it, and wait until all code in the Dispose() is executed. But since the > IsDisposed() return false we for unknow reason hang, does it mean that the > message loop still operates? Or we got some error during "QuitMessageLoop"? It seems in successful run, when the test finish - AwtToolkit::MessageLoop starts - DoQuitMessageLoop is called - AwtToolkit::QuitMessageLoop starts - AwtToolkit::QuitMessageLoop finishes - AwtToolkit::MessageLoop finish - Dispose() is called, m_isDisposed sets to true - shutdown hook isDisposed is true so no infinite loop During unsuccessful run, - AwtToolkit::MessageLoop starts - DoQuitMessageLoop is called - AwtToolkit::QuitMessageLoop starts - AwtToolkit::QuitMessageLoop finishes - AwtToolkit::MessageLoop NEVER ends so Dispose() is not called so m_isDisposed is not set to true so shutdown hook goes in infinite loop. - PR: https://git.openjdk.java.net/jdk/pull/2220
Re: [OpenJDK 2D-Dev] RFR: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory [v3]
On Thu, 28 Jan 2021 08:25:03 GMT, Matthias Baesken wrote:
>> The function AllocateSpaceForGP in freetypeScaler.c calls potentially 2
>> times malloc ; however the memory is not always freed correctly in case of
>> errors.
>> See also the related sonar issue :
>> https://sonarcloud.io/project/issues?id=shipilev_jdk=c=AXck8B_SBBG2CXpcngxr=false=BLOCKER=BUG
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> JDK-8260426
Hi Matthias,
looks fine. See remark below, but that is just nitpicking, this is fine as it
is.
Cheers, Thomas
src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 1297:
> 1295: gpdata->pointCoords = NULL;
> 1296: }
> 1297: return 0;
If you wanted, you could streamline this to:
if (gpdata->pointTypes == NULL || gpdata->pointCoords == NULL) {
free(gpdata->pointTypes);
free(gpdata->pointCoords);
gpdata->pointTypes = gpdata->pointCoords = NULL;
return 0;
Since free(NULL) is valid and a noop.
-
Marked as reviewed by stuefe (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2250
Re: [OpenJDK 2D-Dev] RFR: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory [v3]
> The function AllocateSpaceForGP in freetypeScaler.c calls potentially 2 > times malloc ; however the memory is not always freed correctly in case of > errors. > See also the related sonar issue : > https://sonarcloud.io/project/issues?id=shipilev_jdk=c=AXck8B_SBBG2CXpcngxr=false=BLOCKER=BUG Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: JDK-8260426 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2250/files - new: https://git.openjdk.java.net/jdk/pull/2250/files/afda19e5..70c3e879 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2250=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2250=01-02 Stats: 8 lines in 1 file changed: 6 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2250.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2250/head:pull/2250 PR: https://git.openjdk.java.net/jdk/pull/2250
