On Mon, 2 Mar 2020 12:53:21 GMT, Florian Kirmaier wrote:
>>> It's now part of the systemtests, because the memory-semantics for the
>>> tests in controls is changed due to the TestToolkit.
>>
>> just curious: does that imply that you think the tests in controls ...
>> rather useless?
>
>
On Mon, 2 Mar 2020 10:59:12 GMT, Jeanette Winzenburg
wrote:
>> I've now readded the unit-test. It based on the "InitialNodesMemoryLeakTest".
>>
>> Command to execute: `./gradlew -PFULL_TEST=true -PUSE_ROBOT=true
>> :systemTests:test --tests
>>
On Mon, 2 Mar 2020 10:07:24 GMT, Florian Kirmaier wrote:
>> Rather than removing the test, I was suggesting that you create a test for
>> memory leaks using the same ad hoc approach that our other memory leak tests
>> use. This could later be modified to use the new GC test utility as part of
On Tue, 4 Feb 2020 23:19:17 GMT, Kevin Rushforth wrote:
>> A little bit late ...
>> I have now removed unit-test and it's dependency.
>> I will add a ticket about adding them again.
>
> Rather than removing the test, I was suggesting that you create a test for
> memory leaks using the same ad
On Mon, 27 Jan 2020 15:26:48 GMT, Florian Kirmaier
wrote:
>> The use of static analysis tools to catch certain types of problems is
>> orthogonal to a regression test to validate a bug fix of a specific memory
>> leak.
>>
>> @FlorianKirmaier as mentioned previously, please file a new JBS
On Thu, 19 Dec 2019 15:09:02 GMT, Kevin Rushforth wrote:
>> The pull request has been updated with 1 additional commit.
>
> build.gradle line 2514:
>
>> 2513: compile project(':graphics')
>> 2514: testCompile "de.sandec:JMemoryBuddy:0.1.3"
>> 2515: }
>
> This is a new
On Thu, 19 Dec 2019 15:10:32 GMT, Kevin Rushforth wrote:
>> The pull request has been updated with 1 additional commit.
>
> modules/javafx.graphics/src/main/java/javafx/scene/Parent.java line 1929:
>
>> 1928: */
>> 1929: public List test_getRemoved() {
>> 1930: return removed;
On Mon, 6 Jan 2020 23:05:13 GMT, Kevin Rushforth wrote:
>> I agree. Static analysis tools are quite limited in this regard, and are in
>> now way a substitute for regression testing.
>>
>> So the question is how best to test fixes for memory leaks at runtime. Our
>> current approach can be
On Fri, 20 Dec 2019 13:26:05 GMT, Kevin Rushforth wrote:
>> I highly doubt that a code analysis tool will find such memoryleaks.
>
> I agree. Static analysis tools are quite limited in this regard, and are in
> now way a substitute for regression testing.
>
> So the question is how best to
Accepting that static analysis tools are not best at finding memory
leaks...however they can find some easy to find ones. I was just wondering
if some of these can be run across the code base to find these "easy to
find" ones to help in the process.
Additionally, if there are certain checks that
On Fri, 20 Dec 2019 09:53:56 GMT, Florian Kirmaier
wrote:
>> @dsgrieve
>> It's worth mentioning that JavaFX already has many tests based on
>> System.gc().
>> An advantage of having a testsuit as an library (or copyied from an library)
>> is, that its stability is regulary verified by the
On Thu, 19 Dec 2019 22:22:55 GMT, Florian Kirmaier
wrote:
>> I would urge caution about incorporating JMemoryBuddy without seeking out
>> advice from GC experts. I have my doubts that it will work reliably given
>> its reliance on System.gc(). (Opinion is my own, not my employer's).
>
>
Would it just be better to run some code analysis tools on the code base to
hunt for these? Like maybe sonarqube integratedas part of CI build analysis?
Eric Bresie
ebre...@gmail.com
> On December 19, 2019 at 4:23:14 PM CST, Florian Kirmaier
> wrote:
> On Thu, 19 Dec 2019 19:59:07 GMT, David
On Thu, 19 Dec 2019 19:59:07 GMT, David Grieve wrote:
>> Yes, exactly.
>>
>> @FlorianKirmaier Proposing to add test class to the test infrastructure
>> would be a reasonable alternative to pulling it in as a third-party
>> dependency. I recommend separating it out into its own enhancement,
On Thu, 19 Dec 2019 16:16:28 GMT, Kevin Rushforth wrote:
>> probably not invented so often - just c'd from searches of occurances of
>> System.gc ;-)
>
> Yes, exactly.
>
> @FlorianKirmaier Proposing to add test class to the test infrastructure would
> be a reasonable alternative to pulling
Github failed me. My 'remove' comment was applied to the System.out.println,
not the assert.
> -Original Message-
> From: openjfx-dev On Behalf Of
> David Grieve
> Sent: Thursday, December 19, 2019 2:29 PM
> To: openjfx-dev@openjdk.java.net
> Subject: [EXTERNAL
On Thu, 19 Dec 2019 19:28:34 GMT, Florian Kirmaier
wrote:
>> Hi everyone,
>>
>> ticket: https://bugs.openjdk.java.net/browse/JDK-8236259
>>
>> The fix itself is quite straight forward.
>> It basically just removed the listener which causes the leak.
>>
>> The unit-test for the fix is a bit
On Thu, 19 Dec 2019 15:48:17 GMT, Jeanette Winzenburg
wrote:
>> The point of having this GC-magic in a library is, that it's verified by the
>> library that it's implementation is stable for all common JavaVersions. The
>> attemptGC is basically bad practice. I think in the JavaFX-Codebase
On Thu, 19 Dec 2019 15:44:37 GMT, Florian Kirmaier
wrote:
>> for now, you could have a look at ProgressIndicatorTest (or
>> ProgressSkinTest) attemptGC - might not be optimal (don't know enough about
>> the dirty details of gc :) but gets the test failing/passing before/after
>> fixing a
On Thu, 19 Dec 2019 15:33:09 GMT, Jeanette Winzenburg
wrote:
>> Florian, basically it's a single class isn't it? If so, it might be
>> acceptable to add that class to the test.xx.infrastructure package (don't
>> know if doing so would require a jbs issue)
>
> for now, you could have a look
On Thu, 19 Dec 2019 15:27:59 GMT, Jeanette Winzenburg
wrote:
>> As mentioned, I could copy the class of the library into the JavaFX-Project.
>> Alternatively, I could remove the unit-test entirely.
>>
>> But to be honest, the current state related to memory-leaks in the JavaFX
>> project is
On Thu, 19 Dec 2019 15:12:01 GMT, Kevin Rushforth wrote:
>> The pull request has been updated with 1 additional commit.
>
> I haven't looked at the fix itself, but there are two things that _must_ be
> fixed before this PR can be considered:
> 1. You need to remove the additional 3rd-party
On Thu, 19 Dec 2019 15:24:48 GMT, Florian Kirmaier
wrote:
>> I haven't looked at the fix itself, but there are two things that _must_ be
>> fixed before this PR can be considered:
>> 1. You need to remove the additional 3rd-party test library dependency.
>> 2. You need to revert the change
On Thu, 19 Dec 2019 15:12:12 GMT, Florian Kirmaier
wrote:
>> Hi everyone,
>>
>> ticket: https://bugs.openjdk.java.net/browse/JDK-8236259
>>
>> The fix itself is quite straight forward.
>> It basically just removed the listener which causes the leak.
>>
>> The unit-test for the fix is a bit
On Thu, 19 Dec 2019 14:19:57 GMT, Florian Kirmaier
wrote:
>> just thinking aloud: actually, it's rather whacky that the indicator
>> (mis-)uses the skin's multiplePropertyListener - it's api isn't meant for
>> frequent register/unregisters (the unregisters is quite a heavy measure).
>> Maybe
On Thu, 19 Dec 2019 13:56:39 GMT, Florian Kirmaier
wrote:
>> here's where it was added: https://bugs.openjdk.java.net/browse/JDK-8151617
>
> I don't see an API in the discussion, on how to remove a specific listener.
> Did i miss something? It seems to me, that the API is written to support
On Thu, 19 Dec 2019 14:01:08 GMT, Jeanette Winzenburg
wrote:
>> Yes, that doesn't make a lot of sense. It's probably an artifact because I
>> expected a different API do unregister the listener. I'm gonna change it
>> back!
>
> just thinking aloud: actually, it's rather whacky that the
On Thu, 19 Dec 2019 13:35:27 GMT, Florian Kirmaier
wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java
>> line 521:
>>
>>> 520: registerChangeListener(text.fontProperty(),
>>> (Consumer>) fontListener);
>>> 521:
>>> 522:
On Thu, 19 Dec 2019 13:47:14 GMT, Jeanette Winzenburg
wrote:
>> no way, afaik - there had been debates when the api was added somewhere in
>> jbs, don't recall exactly where
>
> here's where it was added: https://bugs.openjdk.java.net/browse/JDK-8151617
I don't see an API in the discussion,
> Hi everyone,
>
> ticket: https://bugs.openjdk.java.net/browse/JDK-8236259
>
> The fix itself is quite straight forward.
> It basically just removed the listener which causes the leak.
>
> The unit-test for the fix is a bit more complicated.
>
> I added a library JMemoryBuddy
30 matches
Mail list logo
