Identification of Long-Living (Potential Not-DPI-Updated) GCs

[HeikoKlare]

A GC is usually short living, but there is neither a necessity nor any enforcement of that. With multi-monitor HiDPI support, long-living GCs might become a problem, since their zoom value is only initialized once and never updated.

In order to support developers in finding long-living GCs, whose zoom value might need to be updated or whose consumers should be reimplemented using rather short-living GCs, adding some validation logic could be beneficial. With multi-monitor HiDPI support, an option could be to reevaluate once in a while (e.g., upon specific operation calls on the GC) whether the GC's Drawable still has the same zoom as currently stored in the GC's GCData.

On a more technical level (since Drawables do not expose their zoom values), it might be possible to have the GC create a new GC for the drawable and compare it's GCData's zoom value with the current one. In any way, this feature should be something that can be enabled (usually in development mode) by something like a flag / VM argument, as it should not be enabled in productibe use. Maybe the enablement can be combined with the one for #88 (i.e., some kind of "strict mode" for GC).

[fedejeanne]

Is there any such "long-living" GCs that you can mention, @HeikoKlare ? I would like to get a better understanding of the situation here by looking at the code.

Would it be correct to assume that GCs that implement AutoCloseable might be "short living GCs "? I mention this because I found a similar topic in eclipse-platform/eclipse.platform.swt#955 and a possible solution in eclipse-platform/eclipse.platform.swt#1063. The discussion hasn't progressed in months but if we think such approach is valuable and helps the developers use "better" GC's then maybe we can continue the discussion.

[HeikoKlare]

We would like to be able to find GC in our RCP application that are unintentially long-living. The reason is that with the introduction of multi-zoom support, a GC may outlive the context in which it is valid (i.e., the zoom may change as the context control was moved to another monitor). Since we do not know whether we have such GCs, such a setting can help us.

As a comparison to the mentioned discussions: they are rather targeted at properly implementing GCs that are closed again timely, while this one is about detecting ones that are not closed timely in existing code.

[akoch-yatta]

In PaintExample you find an example of a long living GC, although I'm not sure, if it is a very good example.

[fedejeanne]

PaintExample

How can know that the GC has outlived its purpose? If I add this (which is the only useful thing I could find)

... and then resize the window then I always get

GC DPI= Point {96, 96}

... regardless of the monitor in which I try it. I also tried changing the zoom of both monitors to something different i.e. I ended up testing with 2 monitors and 4 zoom levels. Always the same output. I assume this means that the GC doesn't update accordingly and therefore is "long lived"?

[akoch-yatta]

Yes, Device::getDPI doesn't help at all. That's why we try to get rid of it.
I thought a bit about it and I'm not sure how to define that. My initial thought was: If you create two GCs with the same drawables it should behave the same. If that is not the case, the first GC should not be used anymore. Lets make it concrete:

Scenario 1 - Reuse GC

		Canvas canvas = new Canvas(shell, SWT.BORDER);
		canvas.setSize(100, 100);
		GC gc = new GC(canvas);
		Point size1 = gc.textExtent("Font");
		Font font1 = scaleFont(canvas.getFont());
		canvas.setFont(font1);
		Point size2 = gc.textExtent("Font");
		Font font2 = scaleFont(canvas.getFont());
		canvas.setFont(font2);
		Point size3 = gc.textExtent("Font");

		System.out.println(String.format("Height: %s -> %s -> %s", size1.y, size2.y, size3.y));

Output: Height: 21 -> 21 -> 21

Scenario 2 - Recreate GC

		Canvas canvas = new Canvas(shell, SWT.BORDER);
		canvas.setSize(100, 100);
		GC gc = new GC(canvas);
		Point size1 = gc.textExtent("Font");
		Font font1 = scaleFont(canvas.getFont());
		canvas.setFont(font1);
		gc = new GC(canvas);
		Point size2 = gc.textExtent("Font");
		Font font2 = scaleFont(canvas.getFont());
		canvas.setFont(font2);
		gc = new GC(canvas);
		Point size3 = gc.textExtent("Font");

		System.out.println(String.format("Height: %s -> %s -> %s", size1.y, size2.y, size3.y));

Output: Height: 21 -> 25 -> 30

I would say the output is somewhat expected. The font will only be inherited from the Canvas on creation time. After that it has no more effect. So if the underlying state of your drawable changes, that would affect the GC you need to make sure you recreate it in your logic.

What we wanna achieve here is to figure out those scenarios and print a warning when we think there is something wrong. So I would probably propose something like Heiko said:

• Recreate a GC with the same drawable and compare the resulting GCData -> not sure if that really works, there could be something change outside of that, so it could lead to false positives. But I would try it out a bit
• Question is when to do that? Each call would probably be a bit too expensive.

[HeikoKlare]

I'd say that this kind of validation is something you may want to have for debugging purposes. So effects on performance would be acceptable for me as it will only be enabled explicitly at development time.

Maybe an even more simple solution, as indicated in the original post, could be to just start a timer when creating a GC and log a warning in case the GC is not disposed after a specific amount of time (that may be configured as well). That's of course nothing that will give you "exact" results but is rather a simple heuristics.
Why do I think that could be sufficient? We are not so interested in GCs that live "a bit too long" but we would rather like to cover cases where GC are very long living, such as being created for a diagram once and then used throughout a diagram's lifecycle. Then with reasonable thresholds like 20 seconds, you will easily find them. And GCs that live "a bit too long" but still only for few milliseconds or even seconds will not be problem for multi-monitor HiDPI support anyway.
So to keep it as simple as necessary, we may think of such an easy solution.

[fedejeanne]

So if I understand correctly @akoch-yatta , in your Scenario 1 - Reuse GC you have "long lived" GC's which produce always a height of 21 because they don't react to DPI changes (which are perceived in the pseudo-method scaleFont?) whereas in Scenario 2 - Recreate GC you have "short lived" GC's that react to DPI changes. Did I get it right?

If that is the case that I'd say Heiko's approach might too simplistic because such changes can occur way too fast and a timeout might miss some cases.

I'm not saying it's a bad approach, I'm just saying "I wouldn't deliver that approach to the community". @HeikoKlare would it be OK to try your approach in an internal patch first and then see if we can (and want to) build from there? If it's good enough for us at Vector and we can roll out the patch easily enough, it could be a quick win.

[akoch-yatta]

Doesn't necessarily have anything to do with DPI changes. It just outlines how the relationship between Drawable and GC is. So no changes on the Drawables will affect the GC created for it. Changed zoom would be one noticeable effect yes.