"incidental" means "not significant".

If this cached object saved a small amount of work in 5% of cases
when in 95% of cases we need to do that work anyway, then
the cache becomes incidental ...

-phil.

On 5/4/16, 6:51 AM, Sergey Bylokhov wrote:
On 03.05.16 6:37, Philip Race wrote:
On 5/2/16, 2:28 PM, Sergey Bylokhov wrote:
On 03.05.16 0:07, Phil Race wrote:
And this is fine because we use only identity checks inside the
validate method. If the java object was deleted we should update the
native state anyway.

Not sure how to read that. Do you mean there is always
some amount of work to do anyway ?

An example: if we started the paint operation for some surface and set
the clip, then for the next paint operation we skip setting the
surface and clip if they "==" to the previous values. So if the old
data were deleted by GC will mean that we cannot draw to them(and
these cached references are not useful)


So that is a yes .. this is an incidental amount of work ?

I am not sure what you mean by "incidental amount of work"? Can you please clarify.



Is there any value in doing the "nulling" in Anton's earlier version
of the fix to be more prompt ?

The current change is one step-up in the chain of references, and the
nulling in the CGlayer code will leave the leak of surface.


I know it is not sufficient, but is it a worthwhile addition ?

If we will set some data to null, then we will need to update all usage of it to be ready to null. I think that the current solution cover the problem (but there are some other leaks which will be fixed separately).


is BufferedContext only used by D3D & OGL ?

Yes, but I checked direct usage.

What does direct usage mean ?
The test program does not do this.

I meant that, I grep all usages of this class, and all of these usages in d3d/ogl.

Sounds good.

I complete full jck/regression testrun(win+d3d), no new bugs were found.


-phil.


On 05/02/2016 01:26 PM, Sergey Bylokhov wrote:
Hello,
Please review the fix for jdk9.

Bug evaluation was done by Anton:
http://mail.openjdk.java.net/pipermail/awt-dev/2016-April/011177.html

This is a cross-platform bug it affects d3d/ogl pipelines. The
problem
is that BufferedContex cached information to skip some native
reconfigurations. But this cache cause a memory leak if some
data(src/dst surfaces) was cached and there was no new rendering in
this context(we create context per-d3d_device/ogl_config).

In the fix I changed all these caches to weak references. Note that i use a references as initial values instead of null, just to eliminate
the null checks in the body of the method.

The test was updated to be more stable(flushed the EDT + flushed the
Disposer thread).

Bug: https://bugs.openjdk.java.net/browse/JDK-8028486
Webrev can be found at:
http://cr.openjdk.java.net/~serb/8028486/webrev.00









Reply via email to