Re: [OpenJDK 2D-Dev] RFR: 8261533: Java_sun_font_CFont_getCascadeList leaks memory according to Xcode
On Thu, 11 Feb 2021 18:42:30 GMT, Phil Race wrote:
> The various CF*Copy* functions called here need a matching CFRelease
src/java.desktop/macosx/native/libawt_lwawt/font/AWTFont.m line 578:
> 576: (*env)->CallBooleanMethod(env, arrayListOfString, addMID,
> jFontName);
> 577: if ((*env)->ExceptionOccurred(env)) {
> 578: CFRelease(fds);
but DeleteLocalRef(jFontName) shouldn't be called here too?
-
PR: https://git.openjdk.java.net/jdk/pull/2532
Re: [OpenJDK 2D-Dev] RFR: 8261533: Java_sun_font_CFont_getCascadeList leaks memory according to Xcode
On Thu, 11 Feb 2021 18:42:30 GMT, Phil Race wrote: > The various CF*Copy* functions called here need a matching CFRelease src/java.desktop/macosx/native/libawt_lwawt/font/AWTFont.m line 575: > 573: #endif > 574: jstring jFontName = (jstring)NSStringToJavaString(env, fontname); > 575: CFRelease(fontname); Should we call CFRelease or should we call DeleteLocalRef()? - PR: https://git.openjdk.java.net/jdk/pull/2532
Re: [OpenJDK 2D-Dev] RFR: 8261533: Java_sun_font_CFont_getCascadeList leaks memory according to Xcode
On Fri, 12 Feb 2021 04:42:34 GMT, Prasanta Sadhukhan wrote: >> The various CF*Copy* functions called here need a matching CFRelease > > src/java.desktop/macosx/native/libawt_lwawt/font/AWTFont.m line 575: > >> 573: #endif >> 574: jstring jFontName = (jstring)NSStringToJavaString(env, >> fontname); >> 575: CFRelease(fontname); > > Should we call CFRelease or should we call DeleteLocalRef()? OK. I see DeleteLocalRef is called for jFontName.. - PR: https://git.openjdk.java.net/jdk/pull/2532
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
I have worked out how to pass this option to at least the jtreg tests for the lanai headful mach5 job, so once this is fixed we can check it out in jtreg and get some level of confidence that we are checking all the important cases. Note that we know some tests will fail just because it spits out a message that they will mis-parse but it is still worth doing. Need to figure out the same for JCK .. -phil. On 2/11/21 3:12 PM, Phil Race wrote: On Thu, 11 Feb 2021 21:04:16 GMT, Gerard Ziemski wrote: I guess you will only see this if `Metal API Validation Enabled` Which actually begs a question of whether we tested with `Metal API Validation Enabled` ? I submitted https://bugs.openjdk.java.net/browse/JDK-8261620 for this bug. Could be that there are more such issues but since this crash occurs on start up I can't say what else there might be. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Thu, 11 Feb 2021 21:04:16 GMT, Gerard Ziemski wrote: >> I guess you will only see this if `Metal API Validation Enabled` > > Which actually begs a question of whether we tested with `Metal API > Validation Enabled` ? I submitted https://bugs.openjdk.java.net/browse/JDK-8261620 for this bug. Could be that there are more such issues but since this crash occurs on start up I can't say what else there might be. - PR: https://git.openjdk.java.net/jdk/pull/2403
[OpenJDK 2D-Dev] Integrated: 8261282: Lazy initialization of built-in ICC_Profile/ColorSpace classes is too lazy
On Sun, 7 Feb 2021 05:36:52 GMT, Sergey Bylokhov wrote:
> Short fix description:
>
> The fix changes the initialization of the built-in color profiles/spaces from
> the lazy initialization per requested profile via static synchronization to
> the lazy initialization of all profiles/spaces on the first request via class
> initialization(holder idiom).
>
> Long fix description:
>
> 1. A long time ago the factory method ICC_Profile#getInstance() was a
> heavyweight method. For each requested profile it used the data of the icc
> file(ex: sRGB: 6kb and pycc: 200kb).
> As a result, in our code, we have tried to skip this method as much as
> possible or delay its usage. But we found that we have to use it anyway
> during startup. So we implemented the deferral mechanic for the sRGB profile.
> When this profile is requested we did not read the data but return the stub
> that contained some known in advance properties, such as num of color
> components, etc. The icc data is used only if the user request some of it
> later -> in this case we "activate" the profile and drop the stub data.
>
> 2. Later we found that we may need some other profiles at startup, and the
> deferral mechanics was implemented for all profiles by the JDK-6793818. But
> profile activation was implemented as one step for all profiles at once, so
> if one profile such as sRGB was activated then the next profiles returned
> from the "ICC_Profile#getInstance" if not requested before was activated as
> well(used the icc file data).
>
> 3. The deferral mechanics were updated in the JDK-6986863. Now activation of
> one profile does not affect other profiles and as a result, the
> "ICC_Profile#getInstance" always returns the stubs of the requested profiles.
>
> 4. Each profile stub contains just a few lightweight objects, but still, use
> the heavyweight static synchronization to access/create it, see:
>
> https://github.com/openjdk/jdk/blob/9d59dec200490f11bfc1c661b30f10c7edee3a6d/src/java.desktop/share/classes/java/awt/color/ICC_Profile.java#L821
>Note that we have separate blocks for each profile("stub"). That looks an
> overkill.
>
> 5. Note that in theory, it is not necessary to create these stubs lazily,
> each stub is a ICC_Profile object and ProfileDeferralInfo object, so we can
> use them as static fields in the ICC_Profile class. But here that classes
> initialization order come to play -> it is a bad idea to refer to the
> ICC_ProfileRGB(a subclass of the ICC_Profile) in the static block of
> ICC_Profile class because this could cause a deadlock.
>
> 6. So this change merged initialization of all stubs to the one-step, still
> initialize stubs lazily, and maintains the singleton property.
This pull request has now been integrated.
Changeset: bf47a479
Author:Sergey Bylokhov
URL: https://git.openjdk.java.net/jdk/commit/bf47a479
Stats: 370 lines in 6 files changed: 171 ins; 151 del; 48 mod
8261282: Lazy initialization of built-in ICC_Profile/ColorSpace classes is too
lazy
Reviewed-by: azvegint
-
PR: https://git.openjdk.java.net/jdk/pull/2447
Re: [OpenJDK 2D-Dev] RFR: 8261533: Java_sun_font_CFont_getCascadeList leaks memory according to Xcode
On Thu, 11 Feb 2021 18:42:30 GMT, Phil Race wrote: > The various CF*Copy* functions called here need a matching CFRelease Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2532
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Thu, 11 Feb 2021 20:58:36 GMT, Gerard Ziemski wrote: >> src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLLayer.m line 112: >> >>> 110: sourceSize:MTLSizeMake(self.buffer.width, >>> self.buffer.height, 1) >>> 111: toTexture:mtlDrawable.texture destinationSlice:0 >>> destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; >>> 112: [blitEncoder endEncoding]; >> >> There is an issue with this code. Running Java2D.jar in Xcode asserts here >> with this message: >> >> 2021-02-11 14:11:45.710457-0600 java[49971:9486360] Metal API Validation >> Enabled >> 2021-02-11 14:11:46.038720-0600 system_profiler[49975:9486885] Metal API >> Validation Enabled >> -[MTLDebugBlitCommandEncoder >> internalValidateCopyFromTexture:sourceSlice:sourceLevel:sourceOrigin:sourceSize:toTexture:destinationSlice:destinationLevel:destinationOrigin:options:]:474: >> failed assertion `(sourceOrigin.y + sourceSize.height)(444) must be <= >> height(400).' >> (lldb) >> >> and forcing the execution past it results in the java process crashing. >> >> A solution would be to clip the src size like this: >> >> NSUInteger src_x = self.leftInset*self.contentsScale; >> NSUInteger src_y = self.topInset*self.contentsScale; >> NSUInteger src_w = self.buffer.width-src_x; >> NSUInteger src_h = self.buffer.height-src_y; >> >> [blitEncoder >> copyFromTexture:self.buffer sourceSlice:0 sourceLevel:0 >> sourceOrigin:MTLOriginMake(src_x, src_y, 0) >> sourceSize:MTLSizeMake(src_w, src_h, 1) >> toTexture:mtlDrawable.texture destinationSlice:0 >> destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; >> [blitEncoder endEncoding]; > > I guess you will only see this if `Metal API Validation Enabled` Which actually begs a question of whether we tested with `Metal API Validation Enabled` ? - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Thu, 11 Feb 2021 20:55:35 GMT, Gerard Ziemski wrote: >> Ajit Ghaisas has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Lanai PR#181 - 8261143 - aghaisas >> - Lanai PR#180 - 8261546 - jdv > > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLLayer.m line 112: > >> 110: sourceSize:MTLSizeMake(self.buffer.width, >> self.buffer.height, 1) >> 111: toTexture:mtlDrawable.texture destinationSlice:0 >> destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; >> 112: [blitEncoder endEncoding]; > > There is an issue with this code. Running Java2D.jar in Xcode asserts here > with this message: > > 2021-02-11 14:11:45.710457-0600 java[49971:9486360] Metal API Validation > Enabled > 2021-02-11 14:11:46.038720-0600 system_profiler[49975:9486885] Metal API > Validation Enabled > -[MTLDebugBlitCommandEncoder > internalValidateCopyFromTexture:sourceSlice:sourceLevel:sourceOrigin:sourceSize:toTexture:destinationSlice:destinationLevel:destinationOrigin:options:]:474: > failed assertion `(sourceOrigin.y + sourceSize.height)(444) must be <= > height(400).' > (lldb) > > and forcing the execution past it results in the java process crashing. > > A solution would be to clip the src size like this: > > NSUInteger src_x = self.leftInset*self.contentsScale; > NSUInteger src_y = self.topInset*self.contentsScale; > NSUInteger src_w = self.buffer.width-src_x; > NSUInteger src_h = self.buffer.height-src_y; > > [blitEncoder > copyFromTexture:self.buffer sourceSlice:0 sourceLevel:0 > sourceOrigin:MTLOriginMake((jint)src_x, (jint)src_y, 0) > sourceSize:MTLSizeMake(src_w, src_h, 1) > toTexture:mtlDrawable.texture destinationSlice:0 > destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; > [blitEncoder endEncoding]; I guess you will only see this if `Metal API Validation Enabled` - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Thu, 11 Feb 2021 11:51:57 GMT, Ajit Ghaisas wrote: >> **Description :** >> This is the implementation of [JEP 382 : New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361) >> It implements a Java 2D internal rendering pipeline for macOS using the >> Apple Metal API. >> The entire work on this was done under [OpenJDK Project - >> Lanai](http://openjdk.java.net/projects/lanai/) >> >> We iterated through several Early Access (EA) builds and have reached a >> stage where it is ready to be integrated to openjdk/jdk. The latest EA build >> is available at - https://jdk.java.net/lanai/ >> >> A new option -Dsun.java2d.metal=true | True needs to be used to use this >> pipeline. >> >> **Testing :** >> This implementation has been tested with the tests present at - [Test Plan >> for JEP 382: New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396) >> >> **Note to reviewers :** >> 1) Default rendering pipeline on macOS has not been changed by this PR. >> OpenGL still stays as the default rendering pipeline and Metal rendering >> pipeline is optional to choose. >> >> 2) To apply and test this PR - >> To enable the metal pipeline you must specify on command line >> -Dsun.java2d.metal=true (No message will be printed in this case) or >> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is >> enabled gets printed) >> >> 3) Review comments (including some preliminary informal review comments) are >> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 > > Ajit Ghaisas has updated the pull request incrementally with two additional > commits since the last revision: > > - Lanai PR#181 - 8261143 - aghaisas > - Lanai PR#180 - 8261546 - jdv Changes requested by gziemski (Committer). src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLLayer.m line 112: > 110: sourceSize:MTLSizeMake(self.buffer.width, > self.buffer.height, 1) > 111: toTexture:mtlDrawable.texture destinationSlice:0 > destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; > 112: [blitEncoder endEncoding]; There is an issue with this code. Running Java2D.jar in Xcode asserts here with this message: 2021-02-11 14:11:45.710457-0600 java[49971:9486360] Metal API Validation Enabled 2021-02-11 14:11:46.038720-0600 system_profiler[49975:9486885] Metal API Validation Enabled -[MTLDebugBlitCommandEncoder internalValidateCopyFromTexture:sourceSlice:sourceLevel:sourceOrigin:sourceSize:toTexture:destinationSlice:destinationLevel:destinationOrigin:options:]:474: failed assertion `(sourceOrigin.y + sourceSize.height)(444) must be <= height(400).' (lldb) and forcing the execution past it results in the java process crashing. A solution would be to clip the src size like this: NSUInteger src_x = self.leftInset*self.contentsScale; NSUInteger src_y = self.topInset*self.contentsScale; NSUInteger src_w = self.buffer.width-src_x; NSUInteger src_h = self.buffer.height-src_y; [blitEncoder copyFromTexture:self.buffer sourceSlice:0 sourceLevel:0 sourceOrigin:MTLOriginMake((jint)src_x, (jint)src_y, 0) sourceSize:MTLSizeMake(src_w, src_h, 1) toTexture:mtlDrawable.texture destinationSlice:0 destinationLevel:0 destinationOrigin:MTLOriginMake(0, 0, 0)]; [blitEncoder endEncoding]; - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] RFR: 8260695: The java.awt.color.ICC_Profile#getData/getData(int) are not thread safe [v3]
> Both methods are implemented in a similar way. > 1. Requests the size of the profile/tag data > 2. Creates an array of the correct size > 3. Requests the data and copy it to the array > > If the data will be changed concurrently between steps 2. and 3. then we will > get a mismatch between the array and copied data. > > In the fix, all steps above are merged to just one step - return the data > when requested. Sergey Bylokhov 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 eight additional commits since the last revision: - Merge branch 'master' into JDK-8260695 - Merge branch 'master' into JDK-8260695 - cleanup - Merge branch 'JDK-8260695' of https://github.com/mrserb/jdk into JDK-8260695 - Update LCMSProfile.java - Update LCMSProfile.java - Create MTGetData.java - Initial fix - Changes: - all: https://git.openjdk.java.net/jdk/pull/2330/files - new: https://git.openjdk.java.net/jdk/pull/2330/files/a9347ae3..24211666 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2330&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2330&range=01-02 Stats: 15435 lines in 405 files changed: 8177 ins; 5003 del; 2255 mod Patch: https://git.openjdk.java.net/jdk/pull/2330.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2330/head:pull/2330 PR: https://git.openjdk.java.net/jdk/pull/2330
Re: [OpenJDK 2D-Dev] RFR: 8261533: Java_sun_font_CFont_getCascadeList leaks memory according to Xcode
On Thu, 11 Feb 2021 19:45:07 GMT, Sergey Bylokhov wrote: > Is it a long-standing bug or caused by the JNF removal? long-standing. - PR: https://git.openjdk.java.net/jdk/pull/2532
Re: [OpenJDK 2D-Dev] RFR: 8261533: Java_sun_font_CFont_getCascadeList leaks memory according to Xcode
On Thu, 11 Feb 2021 18:42:30 GMT, Phil Race wrote: > The various CF*Copy* functions called here need a matching CFRelease Is it a long-standing bug or caused by the JNF removal? - PR: https://git.openjdk.java.net/jdk/pull/2532
[OpenJDK 2D-Dev] RFR: 8261533: Java_sun_font_CFont_getCascadeList leaks memory according to Xcode
The various CF*Copy* functions called here need a matching CFRelease - Commit messages: - 8261533: ava_sun_font_CFont_getCascadeList leaks memory according to Xcode Instruments leak profile Changes: https://git.openjdk.java.net/jdk/pull/2532/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2532&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8261533 Stats: 9 lines in 1 file changed: 8 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2532.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2532/head:pull/2532 PR: https://git.openjdk.java.net/jdk/pull/2532
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v5]
> The API doc for Graphics2D.clip(shape s) claims that passing a null argument > would actually clear the existing clipping area, which is incorrect. > This statement is applicable only to G2D.setClip() and not for the clip() > method. G2D.clip() would throw a NullPointerException when it encounters a > null argument. > Updated spec to rectify this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Review comment fix. Test updated - Changes: - all: https://git.openjdk.java.net/jdk/pull/2476/files - new: https://git.openjdk.java.net/jdk/pull/2476/files/e4053de8..51fbe247 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=03-04 Stats: 5 lines in 3 files changed: 2 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2476.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2476/head:pull/2476 PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v4]
On Thu, 11 Feb 2021 04:14:54 GMT, Prasanta Sadhukhan
wrote:
>> The API doc for Graphics2D.clip(shape s) claims that passing a null argument
>> would actually clear the existing clipping area, which is incorrect.
>> This statement is applicable only to G2D.setClip() and not for the clip()
>> method. G2D.clip() would throw a NullPointerException when it encounters a
>> null argument.
>> Updated spec to rectify this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix as per Phil's comment
test/jdk/java/awt/Graphics2D/TestNullClip.java line 41:
> 39:
> 40: g2d.setClip(0, 0, 100, 100);
> 41: Shape clip = g2d.getClip();
Looks like the `clip` variable is not used.
Also I've just noticed that the years are not updated in copyright headers of
Graphics and Graphics2d.
test/jdk/java/awt/Graphics2D/TestNullClip.java line 46:
> 44: if (clip1 != null) {
> 45: throw new RuntimeException("Clip is not cleared");
> 46: }
There is no check that `g2d.clip(null);` call does not throw NPE when no user
clip is set.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
> **Description :** > This is the implementation of [JEP 382 : New macOS Rendering > Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361) > It implements a Java 2D internal rendering pipeline for macOS using the Apple > Metal API. > The entire work on this was done under [OpenJDK Project - > Lanai](http://openjdk.java.net/projects/lanai/) > > We iterated through several Early Access (EA) builds and have reached a stage > where it is ready to be integrated to openjdk/jdk. The latest EA build is > available at - https://jdk.java.net/lanai/ > > A new option -Dsun.java2d.metal=true | True needs to be used to use this > pipeline. > > **Testing :** > This implementation has been tested with the tests present at - [Test Plan > for JEP 382: New macOS Rendering > Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396) > > **Note to reviewers :** > 1) Default rendering pipeline on macOS has not been changed by this PR. > OpenGL still stays as the default rendering pipeline and Metal rendering > pipeline is optional to choose. > > 2) To apply and test this PR - > To enable the metal pipeline you must specify on command line > -Dsun.java2d.metal=true (No message will be printed in this case) or > -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is > enabled gets printed) > > 3) Review comments (including some preliminary informal review comments) are > tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 Ajit Ghaisas has updated the pull request incrementally with two additional commits since the last revision: - Lanai PR#181 - 8261143 - aghaisas - Lanai PR#180 - 8261546 - jdv - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/6a3f96ef..7340d067 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2403&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2403&range=05-06 Stats: 82 lines in 2 files changed: 0 ins; 54 del; 28 mod Patch: https://git.openjdk.java.net/jdk/pull/2403.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2403/head:pull/2403 PR: https://git.openjdk.java.net/jdk/pull/2403
