On Thu, 3 Dec 2020 23:52:25 GMT, Sergey Bylokhov <s...@openjdk.org> 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. src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 1064: > 1062: ProfileDeferralInfo info = deferralInfo; > 1063: if (info != null) { > 1064: return info.profileClass; Separate MT issue found by the code review, we can get NPE here if deferralInfo will be set to null(activated) after the null-check. Proved by the ProfileActivationDuringpropertyAccess test. src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java line 72: > 70: int transformType) > 71: { > 72: /* Actually, it is not a complete transform but just part of it */ We have to activate it here because later in this class we call: 76 lcmsProfiles[0] = LCMS.getProfileID(profile); And that native method expects the activated profile, after the fix, we will activate it in that native method. src/java.desktop/share/native/liblcms/LCMS.c line 652: > 650: return NULL; > 651: } > 652: The purpose of this method is to bypass java access control and access the private ICC_Profile.cmmProfile field, I have added a call for activation here as well. This could be improved someday by the accessor or something like this. src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 1001: > 999: * was ignored during activation. > 1000: */ > 1001: private void activate() { As of now, this code block is never executed, since we never pass ProfileDeferralInfo as an InputStream. probably we did it a long time ago. src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java line 83: > 81: > 82: /** > 83: * Constructs a new ColorConvertOp which will convert Seems unnecessary, this class uses the `sun.java2d.cmm.ColorTransform` which will load the profile when necessary. src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 782: > 780: if ((getColorSpaceType (p) == ColorSpace.TYPE_GRAY) && > 781: (getData (p, icSigMediaWhitePointTag) != null) && > 782: (getData (p, icSigGrayTRCTag) != null)) { Why do we need to load standard profiles if the user wants to load some other profile from the data array? ------------- PR: https://git.openjdk.java.net/jdk/pull/1613
