Improve performance of icon loading in ImageUtilities

[eirikbakke]

Body

With NetBeans now containing more SVG icons, and with more icons being loaded via ImageUtilities rather than alternative methods, it might be worth taking a look at the performance of ImageUtilities.getIcon, and see if there are possible optimizations that should be done. A first step, switching from Batik to JSVG, was already done.

This issue is intended to continue the discussion that was started in a comment in another issue.

Committer

• I acknowledge that I am a maintainer/committer in the Apache NetBeans project.

[eirikbakke]

(Repeating my profiling results from the other thread:) Here's a quick profile of ImageUtilities.getIcon on NetBeans startup, with two maven projects and a few Java editor windows open:

This is after the switch to JSVG and after the two big ImageIcon patches have been applied.

Note that this is just for a single run. Though the proportions seem consistent from run to run.

(Repeating again results from the other thread.) The many calls to getResource are due to many suffix variations attempted for each icon base path. For example, here are all the non-existent variations that are attempted to be loaded, excluding SVG extensions, for a single icon:

org/netbeans/modules/editor/bookmarks/resources/bookmark_16_nb_en_US.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_nb_en.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_nb.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_en_US.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_en.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_pressed_nb_en_US.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_pressed_nb_en.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_pressed_nb.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_pressed_en_US.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_pressed_en.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_pressed.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_rollover_nb_en_US.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_rollover_nb_en.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_rollover_nb.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_rollover_en_US.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_rollover_en.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_rollover.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_disabled_nb_en_US.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_disabled_nb_en.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_disabled_nb.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_disabled_en_US.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_disabled_en.png
org/netbeans/modules/editor/bookmarks/resources/bookmark_16_disabled.png

There are many duplicate SVG icons in the NetBeans repo, but this does not appear to contribute significantly to startup time. Most of the icons loaded at startup are in fact distinct images.

[mbien]

since the point was brought up that all (or most) icons should be loaded with localized=true:

I don't have anything against this, but please wait a bit with this change till things settled. The localized code path is more complex and has more indirections, lets not change too many things at once ;) (there is also some potential for a cleanup of ImageUtilities impl before more things are changed - have something in my stash, planning to open a PR at some point)

regarding image loading performance:

it is something worth having an eye on, but me measuring #8194 (comment) didn't mean that i felt performance was insufficient. It also doesn't even mean that it influences startup, the bottleneck could be somewhere else and this happens in parallel decoupled from the bottleneck. Lets balance complexity with effect - there is a chance that everything will be still fine even after everything calls into ImageUtilities (uses localization) and without extra optimizations.

edit: linking to closed conversations doesn't seem to work, here the content of the linked comment:

---

mostly EDT as of today (pending PRs not applied! - this has to be repeated):

   7465 thread: Thread[AWT-EventQueue-0,6,IDE Main]
    324 thread: Thread[Folder Instance Processor,1,system]
     29 thread: Thread[main,5,IDE Main]
     16 thread: Thread[Warm Up,1,system]
      4 thread: Thread[org.netbeans.modules.web.clientproject.browser.ActiveBrowserAction,1,system]

removing the critical section wouldn't change much at the moment.

another metric: about 450ms is spent for loading icons during startup

[eirikbakke]

please wait a bit with this change till things settled.

Could you clarify which change you are referring to here? Optimizing ImageUtilities, changing localized=true/false defaults, or merging one of the existing PRs?

[mbien]

please wait a bit with this change till things settled.

Could you clarify which change you are referring to here? Optimizing ImageUtilities, changing localized=true/false defaults, or merging one of the existing PRs?

second paragraph is referring to: "since the point was brought up that all (or most) icons should be loaded with localized=true"

Which I don't have anything against, but I do think should happen after the global ImageUtilities PRs are merged. While doing this we should have an eye on startup performance but I honestly don't really expect a noticeable difference there.

[eirikbakke]

Thanks, understood! Yeah, I wasn't planning any immediate work on this myself; it was just good to have the discussion around for the future.

[neilcsmith-net]

second paragraph is referring to: "since the point was brought up that all (or most) icons should be loaded with localized=true"

Given that point was made by me, let's clarify what was meant. 😄 All icons / images should be brandable. I agree with Eirik that very few images actually need to be localizable.

At this point, I wouldn't look to fix the usages across the code base. We might just consider existing uses on a case by case basis if a downstream consumer complains about un-brandable usage.

If we do want to look at this from a branding perspective, or there's evidence it's necessary from a performance perspective, I would look at ImageUtilities itself. Ensure branding tokens are taken into account on the non-localized path, then make localization more opt-in. That would reduce the number of suffixes that are searched quite significantly. It's also a minor behaviour change, but one possibly worth considering.

EDIT: or alternatively make locale suffixes dependent on a system property that can be enabled in the platform if required. Do we actually need locale suffixes anywhere in the IDE?