8261547: Minor memory leak reported for Metal Rendering pipeline #182

prsadhuk · 2021-02-11T13:03:01Z

Memory leak fix for couple of strings.

Progress

Change must not contain extraneous whitespace

Issue

JDK-8261547: Minor memory leak reported for Metal Rendering pipeline

Reviewers

Kevin Rushforth (@kevinrushforth - Committer)
Ajit Ghaisas (@aghaisas - Committer)

Download

$ git fetch https://git.openjdk.java.net/lanai pull/182/head:pull/182
$ git checkout pull/182

bridgekeeper · 2021-02-11T13:03:44Z

👋 Welcome back psadhukhan! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2021-02-11T13:04:21Z

@prsadhuk this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8261547
git fetch https://git.openjdk.java.net/lanai master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

mlbridge · 2021-02-11T13:07:00Z

Webrevs

openjdk · 2021-02-11T13:23:52Z

@prsadhuk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8261547: Minor memory leak reported for Metal Rendering pipeline

Reviewed-by: kcr, aghaisas

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 19 new commits pushed to the master branch:

5c73963: Automatic merge of jdk:master into master
60a2072: 8260431: com/sun/jdi/JdbOptions.java failed with "RuntimeException: 'prop[boo] = >foo<' missing from stdout/stderr"
bf47a47: 8261282: Lazy initialization of built-in ICC_Profile/ColorSpace classes is too lazy
f4cfd75: 8261510: Use RFC numbers and protocol titles in sun.security.ssl.SSLExtension comments
75c8489: 8261604: ProblemList jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java
1740de2: 8261297: NMT: Final report should use scale 1
c342323: 8261431: SA: Add comments about load address of executable
4a72cea: 8261509: Move per-thread StackWatermark from Thread to JavaThread class
eef86a8: 8261029: Code heap page sizes not traced correctly using os::trace_page_sizes
0a89987: 8240281: Remove failing assertion code when selecting first memory state in SuperWord::co_locate_pack
... and 9 more: https://git.openjdk.java.net/lanai/compare/addfb6a977c03b8ae29e057bd18165a67b0e0278...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

kevinrushforth · 2021-02-11T13:24:44Z

@prsadhuk you forgot the bug ID in the title of this PR (and thus the commit message)

kevinrushforth · 2021-02-11T17:03:47Z

src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m

@@ -117,14 +117,16 @@
    JNI_COCOA_ENTER(env);
    NSMutableArray * retArray = [NSMutableArray arrayWithCapacity:3];
    [retArray addObject: [NSNumber numberWithInt: (int)displayID]];
-    [retArray addObject: [NSString stringWithUTF8String: JNU_GetStringPlatformChars(env, shadersLibName, 0)]];
+    NSString *str = JNU_GetStringPlatformChars(env, shadersLibName, 0);


This should be of type char * not NSString *

kevinrushforth · 2021-02-11T17:04:10Z

src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m

@@ -148,14 +150,16 @@
    JNI_COCOA_ENTER(env);
    NSMutableArray * retArray = [NSMutableArray arrayWithCapacity:3];
    [retArray addObject: [NSNumber numberWithInt: (int)displayID]];
-    [retArray addObject: [NSString stringWithUTF8String: JNU_GetStringPlatformChars(env, mtlShadersLib, 0)]];
+    NSString *str = JNU_GetStringPlatformChars(env, mtlShadersLib, 0);


kevinrushforth · 2021-02-11T17:14:40Z

It runs fine for me on dual screens. I verified that it is hitting the call to JNU_ReleaseStringPlatformChars. The only issue is the type of str, which should be char * (or maybe const char *) as noted above.

prsadhuk · 2021-02-11T17:24:55Z

SwingSet2, J2DDemo works fine with this change.

aghaisas · 2021-02-12T04:47:22Z

I tested J2DDemo and SwingSet2 with graphics card switch from integrated to discrete GPU and other way round. No regression is observed.

prsadhuk · 2021-02-12T04:56:34Z

/integrate

openjdk · 2021-02-12T04:57:22Z

@prsadhuk Since your change was applied there have been 19 commits pushed to the master branch:

5c73963: Automatic merge of jdk:master into master
60a2072: 8260431: com/sun/jdi/JdbOptions.java failed with "RuntimeException: 'prop[boo] = >foo<' missing from stdout/stderr"
bf47a47: 8261282: Lazy initialization of built-in ICC_Profile/ColorSpace classes is too lazy
f4cfd75: 8261510: Use RFC numbers and protocol titles in sun.security.ssl.SSLExtension comments
75c8489: 8261604: ProblemList jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java
1740de2: 8261297: NMT: Final report should use scale 1
c342323: 8261431: SA: Add comments about load address of executable
4a72cea: 8261509: Move per-thread StackWatermark from Thread to JavaThread class
eef86a8: 8261029: Code heap page sizes not traced correctly using os::trace_page_sizes
0a89987: 8240281: Remove failing assertion code when selecting first memory state in SuperWord::co_locate_pack
... and 9 more: https://git.openjdk.java.net/lanai/compare/addfb6a977c03b8ae29e057bd18165a67b0e0278...master

Your commit was automatically rebased without conflicts.

Pushed as commit 52c97e5.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Memory leak fix

13f60be

openjdk bot added merge-conflict Pull request has a merge conflict with target branch rfr Pull request is ready for review labels Feb 11, 2021

openjdk bot added ready Pull request is ready to be integrated and removed merge-conflict Pull request has a merge conflict with target branch labels Feb 11, 2021

Merge master

fe4cedf

prsadhuk changed the title ~~Memory leak fix~~ 8261547: Memory leak fix Feb 11, 2021

openjdk bot removed the ready Pull request is ready to be integrated label Feb 11, 2021

prsadhuk changed the title ~~8261547: Memory leak fix~~ 8261547: Minor memory leak reported for Metal Rendering pipeline Feb 11, 2021

openjdk bot added the ready Pull request is ready to be integrated label Feb 11, 2021

kevinrushforth suggested changes Feb 11, 2021

View reviewed changes

Change to char*

29cb44b

kevinrushforth approved these changes Feb 11, 2021

View reviewed changes

aghaisas approved these changes Feb 12, 2021

View reviewed changes

openjdk bot closed this Feb 12, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8261547: Minor memory leak reported for Metal Rendering pipeline #182

8261547: Minor memory leak reported for Metal Rendering pipeline #182

prsadhuk commented Feb 11, 2021 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 11, 2021

openjdk bot commented Feb 11, 2021

mlbridge bot commented Feb 11, 2021 •

edited

Loading

openjdk bot commented Feb 11, 2021 •

edited

Loading

kevinrushforth commented Feb 11, 2021

kevinrushforth Feb 11, 2021

kevinrushforth Feb 11, 2021

kevinrushforth commented Feb 11, 2021

prsadhuk commented Feb 11, 2021

aghaisas commented Feb 12, 2021 •

edited

Loading

prsadhuk commented Feb 12, 2021

openjdk bot commented Feb 12, 2021

8261547: Minor memory leak reported for Metal Rendering pipeline #182

8261547: Minor memory leak reported for Metal Rendering pipeline #182

Conversation

prsadhuk commented Feb 11, 2021 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Feb 11, 2021

openjdk bot commented Feb 11, 2021

mlbridge bot commented Feb 11, 2021 • edited Loading

Webrevs

openjdk bot commented Feb 11, 2021 • edited Loading

kevinrushforth commented Feb 11, 2021

kevinrushforth Feb 11, 2021

Choose a reason for hiding this comment

kevinrushforth Feb 11, 2021

Choose a reason for hiding this comment

kevinrushforth commented Feb 11, 2021

prsadhuk commented Feb 11, 2021

aghaisas commented Feb 12, 2021 • edited Loading

prsadhuk commented Feb 12, 2021

openjdk bot commented Feb 12, 2021

prsadhuk commented Feb 11, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Feb 11, 2021 •

edited

Loading

openjdk bot commented Feb 11, 2021 •

edited

Loading

aghaisas commented Feb 12, 2021 •

edited

Loading