Skip to content

[Win32] Copy patterns in GC only when necessary#3317

Merged
HeikoKlare merged 1 commit into
eclipse-platform:masterfrom
HeikoKlare:win32-gc-pattern-copy-on-demand
May 30, 2026
Merged

[Win32] Copy patterns in GC only when necessary#3317
HeikoKlare merged 1 commit into
eclipse-platform:masterfrom
HeikoKlare:win32-gc-pattern-copy-on-demand

Conversation

@HeikoKlare
Copy link
Copy Markdown
Contributor

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

Test Results (win32)

   30 files  +    1     30 suites  +1   4m 30s ⏱️ + 4m 26s
4 697 tests +3 682  4 622 ✅ +3 620  75 💤 +62  0 ❌ ±0 
1 227 runs  +  559  1 203 ✅ +  541  24 💤 +18  0 ❌ ±0 

Results for commit 686f727. ± Comparison against base commit e95edb8.

♻️ This comment has been updated with latest results.

if (pattern != null && pattern.isDisposed()) {
// recreate locally managed pattern if the original was disposed
pattern = pattern.copy();
registerForDisposal(pattern);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
eclipse-platform#3296
@HeikoKlare HeikoKlare force-pushed the win32-gc-pattern-copy-on-demand branch from d47b225 to 686f727 Compare May 30, 2026 09:13
@HeikoKlare HeikoKlare merged commit dc0c677 into eclipse-platform:master May 30, 2026
18 checks passed
@HeikoKlare HeikoKlare deleted the win32-gc-pattern-copy-on-demand branch May 30, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants