Community
Participate
Working Groups
I had a memory monitor plugin that graphed the available memory over time. I wrote it using java.util.Timer. The graph looked flat. I re-implemented it using UIJob. The graph became a saw-tooth. The following code illustrates the problem. Create a plugin project. Add the org.eclipse.ui.ActionSets extension and choose the Hello World example. In the run method, add the following code: UIJob j = new UIJob("Tester"){ private int count = 10000; public IStatus runInUIThread(IProgressMonitor monitor) { long total = Runtime.getRuntime().totalMemory()/1024; long free = Runtime.getRuntime().freeMemory()/1024; System.out.println("Total= "+total+" free= "+free); if (--count > 0) schedule(1000); return Status.OK_STATUS; }}; j.schedule(1000); Run the workbench and run the action set. The output should be something like this: Total= # free= # The free value should not be changing. In this instance it is going down continually until there is a GC when it jumps back up.
I will profile to see the severity, but some object creations are expected here. For the graph to be completely flat it would have to be implemented without any object creations. Since a UIJob reports progress feedback in the status line, including animation and messages, this isn't practical. Even the System.out.println here creates several garbage objects that will consume memory (depending on scavenging GC characteristics of the VM being used).
Three are three principal areas responsible for most garbage creation when running this sample program: 1) Many ProgressManager methods use HashSet objects as temporary structures. Each HashSet contains a HashMap object, and Entry[] object, and an Entry object for each item that is added. Iterating over HashSets creates several extra objects. In several cases a Set is not needed and an ArrayList could be used instead, avoiding most of the object creations. Related ProgressManager methods are: getRootElements, addListenerToFamily, removeListener, busyListenersForJob (especially the system job case can easily be optimized). I ran the job 85 times and this created the following objects: 346 HashSet 346 HashMap 368 HashMap$Entry 660 HashMap$KeyIterator 317 HashMap$KeySet 2) ProgressManager.listeners is copied each time someone needs to iterate over them - typically four times per job execution as it goes through the stages of the job lifecycle (scheduled, aboutToRun, running, done). Listener list could instead be copied when a listener is added or removed rather than on iteration (see org.eclipse.core.runtime ListenerList as an example). This iteration creates many Object[] instances (see attached profile output). 3) FinishedJobs.getJobInfos creates two Object[] instances and a Comparator, even though there are no kept jobs in this case. Starting the method with something like the following should eliminate all of these objects in most cases: if (keptjobinfos.isEmpty()) return EMPTY_INFOS; where EMPTY_INFOS is a private static final JobTreeElement[] of size zero.
Created attachment 16052 [details] Allocation backtrace of HashSet instances
Moving to UI since the majority of garbage objects are created there. This is really only a minor performance issue.
Comment/Q on this text: Since a UIJob reports progress feedback in the status line, including animation and messages, this isn't practical. How is a UIJob reporting feedback? Way back in Mx days of 3.0 a UIJob could report feedback (the blue progress bar - min the middle of the status line back then, moved for a UIJob). But we don't see such feedback anymore in the final 3.0 build. Even if a UIJob uses the monitor to define work/report progress nothing is visualized to the user. At least my fiddling with the API that used to report progress no longer does in 3.0.1. If we can't see it - should it be done in the code?
Re comment #5: Because UIJob executes in the UI thread, the UI is essentially dead for that period. No other painting or updates can occur. For this reason there is effectively no progress shown for UIJobs. We don't currently optimize this case (we actually build all the progres infrastructure but it just never paints).
Created attachment 17929 [details] Patch making the changes John suggests
Upping to P2 as I now have a patch
Adding my name to the cc list as we are now tracking performance issues more closely. Please remove the performance keyword if this is not a performance bug.
Running the patch with a job rescheduled jobs resulted in the following stats Patched version 202HashSet 189 HashMap 529 HashMap$Entry 860 HashMap$KeyIterator 133 HashMap$KeySet 3.1 M6 1033HashSet 972 HashMap 603 HashMap$Entry 1796 HashMap$KeyIterator 947 HashMap$KeySet
Patch released for build >20050405
I have a few questions/suggestions on the patch: - suggest using Collections.EMPTY_LIST or Collections.EMPTY_SET for the empty collection, since new ArrayList(0) is not immutable - listeners are being notified while holding a lock, which can potentially lead to deadlocks - instead of: Iterator families = familyListeners.keySet().iterator(); if(!families.hasNext()) return EMPTY_COLLECTION; suggest using: if(families.isEmpty()) return EMPTY_COLLECTION; since it creates less garbage. Also, should consider using the JFace ListenerList, or copying the one from runtime. They have different performance characteristics. The one in runtime copies on write, the JFace one copies on read, but creates no garbage if empty.
Thanks for the tips Nick - I'll apply them now. The problem with using the JFace listener lists is that the JFace one is not synchonized. See Bug 91598 for a discussion of this issue.
Suggested changes released for build >20050415
Verified in 20050510
