On 12/04/16 23:34, Semyon Sadetsky wrote:
Hi Alexander,
Your comparison makes sense.
In addition to your change I'd suggest to remove the reset() method in
SynthContext and set the fields in the constructor.
Does the factory method really necessary? It seems the public
constructor has the same arguments and produces the same object.
This is the good point.
The public constructor has a restriction that a component, region, and
style arguments must not be null.
Some SynthUI classes (for example SynthLabelUI) violates this
restriction calling the SynthContext.getContext(...) method. This can
really be a bug but I would prefer to not mix it with the current fix.
I have created an issue on it: JDK-8154109 Some SynthUI classes can
request to create a SynthContext with null fields
https://bugs.openjdk.java.net/browse/JDK-8154109
Thanks,
Alexandr.
--Semyon
On 4/12/2016 10:02 PM, Alexander Scherbatiy wrote:
Hello,
Could you review the fix:
bug: https://bugs.openjdk.java.net/browse/JDK-8132791
webrev: http://cr.openjdk.java.net/~alexsch/8132791/webrev.00
SynthContext is an object with 4 fields which is used a lot by Synth
L&F for components painting. It has an internal API which allows to
put unused objects to queue and reuse them later. Application
developers will not be able to have access to this API in JDK 9 with
modularization feature so they only be able to create new SyntContext
objects. There are ways that a SyntContext object created by a
developer can be disposed by Synth L&F and put to the unused object
queue. This can lead to a memory leak.
There are 2 options to fix the issue: provide a public API for the
SyntContext object custom disposing or just get rid of the unused
object queue.
Effective Java Guide has a suggestion: “avoiding object creation by
maintaining your own object pool is a bad idea unless the objects in
the pool are extremely heavyweight”
To check if there are memory or performance degradation for the
second solution I used SwingMark application which extensively
creates and paints a lot of Swing components like labels, menus and
others.
The tests were run 5 times with using the object queue and without it.
There results can be found at:
http://cr.openjdk.java.net/~alexsch/8132791/profiling/results_3-5
The minimum and maximum values from 5 runs are:
Tests which used an object queue:
used memory (in bytes)
min: 3.75 million max: 3.78 million
heap size (in bytes)
min: 11.53 million max: 12.58 million
elapsed time (in milliseconds)
min: 57.27 thousand max: 57.97 thousand
requests for SynthContext objects: 380 thousands
created objects: 21
Tests which did not use an object queue:
used memory (in bytes)
min: 3.71 million max: 3.79 million
heap size (in bytes)
min: 11.53 million max: 12.58 million
elapsed time (in milliseconds)
min: 57.90 thousand max: 57.97 thousand
requests for SynthContext objects: 380 thousands
created objects: 380 thousands
There were about 380 thousand request for SynthContext objects in
both tests but only 21 object were crated for the first ones.
However, the used memory size and running time are nearly the same
for both runs for these particular tests.
According to the test results it looks reasonable to remove the
custom object pool support from the SynthContext object.
Thanks,
Alexandr.
