[Win32] Copy patterns in GC only when necessary#3317
Conversation
GC operations for background/foreground Pattern currently copy the passed pattern upon creation. In case the GC is short living and the passed Pattern outlives it, this copy (including a handle allocation) is unnecessary. With this change, the passed Pattern is only copied/recreated when it became disposed. Contributes to eclipse-platform#3296
| if (pattern != null && pattern.isDisposed()) { | ||
| // recreate locally managed pattern if the original was disposed | ||
| pattern = pattern.copy(); | ||
| registerForDisposal(pattern); |
There was a problem hiding this comment.
Is this enough to make sure that the newly "resuscitated" pattern is promptly disposed? I mean it if the original pattern (the one received and stored in the constructor) has already been disposed this means that the caller wanted to free-up some resources, but resuscitating the pattern and registering it for disposal would mean that only when the gc is disposed would the resuscitated pattern also be disposed.
What about:
if (pattern != null && pattern.isDisposed()) {
SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
}?
That would mean that a long-lived GC can't apply a short-lived Pattern.
Alternatively, keeping this code as it is right now but immediately disposing the newly resuscitated Pattern after applying the operation (in apply()) could also free-up resources faster (at the cost of having to resuscitate them and dispose them on every zoom change).
There was a problem hiding this comment.
This change is just an optimization of the original approach to copy the pattern already when creating the operation. We need to copy the pattern and preserve it as long as the GC exists because the operations can be reapplied at any later point in time. Throwing an error in case someone draws the underlying image to a context with a different because the pattern was meanwhile disposed would be unexpected an break the GC's contract (as this use case is properly fine from user perspective).
So this change is just a preparation for:
as with that change we want to dispose the operation directly after applying how, but the pattern has to live longer as it is stored int he GCData object and may be used by later GC operations. Thus only making a copy in case the original pattern is disposed both leads to an easy solution for allowing that follow-up change and also reduces unnecessary copies and memory consumption in every case.
GC operations for background/foreground Pattern currently copy the passed pattern upon creation. In case the GC is short living and the passed Pattern outlives it, this copy (including a handle allocation) is unnecessary.
With this change, the passed Pattern is only copied/recreated when it became disposed.
Contributes to #3296