Community
Participate
Working Groups
Build 3.0.1. This is a very small improvement, but it was large enough to show up on my list of potential optimizations. Essentially it avoids re-reading the feature INI's on startup. Our product has LOTS of feature plug-ins (103), so this adds up. Attached is the proposed change; it isn't sophisticated, essentially saying "if I had at least one tips and tricks / welcome page before, I probably have one now". To be more robust, the stored preference would have to be cleared on a -clean. The net savings on a 3GHz P4 / 2G RAM was 181ms for multiple warm runs. It obviously should be a tad larger for cold runs on a less performant machine.
Created attachment 14813 [details] Modified WorkbenchActionBuilder
Created attachment 14814 [details] Chart shown performance improvement
Created attachment 14815 [details] Correctly modified WorkbenchActionBuilder
OK, a 2% improvement in startup time isn't much. But add four or five of them and it becomes noticeable. Isn't this worth putting in? The correction is included for your convenience...
TC, potential performance test here, as well as a fix for a bug.
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.
Tod, I confirm this is indeed a (small) performance issue.
Marking 3.1 as we may not get to it this milestone
Is this change going in?
We'll decide in the M7 timeframe
This version can't go in because it is so radically different from what we have in this class now. I tried comparing it to the 3.0.2 version but the compare editor said there were too many different. Dan could you give us an idea of where to start?
Search for "dbk" (or FIXME) in the attached file. I've copy/pasted what I believe is the whole tamale: //FIXME dbk performance optimization - don't read ini files for about info IPreferenceStore ps = IDEWorkbenchPlugin.getDefault ().getPreferenceStore(); AboutInfo[] infos = null; if (ps.getBoolean(IDEActionFactory.QUICK_START.getId())) { quickStartAction = IDEActionFactory.QUICK_START.create (getWindow()); registerGlobalAction(quickStartAction); } else { infos = IDEWorkbenchPlugin.getDefault().getFeatureInfos (); // See if a welcome page is specified for (int i = 0; i < infos.length; i++) { if (infos[i].getWelcomePageURL() != null) { quickStartAction = IDEActionFactory.QUICK_START.create(getWindow()); registerGlobalAction(quickStartAction); ps.setValue (IDEActionFactory.QUICK_START.getId(), true); break; } } } if (ps.getBoolean(IDEActionFactory.TIPS_AND_TRICKS.getId())) { tipsAndTricksAction = IDEActionFactory.TIPS_AND_TRICKS.create(getWindow()); registerGlobalAction(tipsAndTricksAction); } else { if (infos == null) infos = IDEWorkbenchPlugin.getDefault ().getFeatureInfos(); // See if a tips and tricks page is specified for (int i = 0; i < infos.length; i++) { if (infos[i].getTipsAndTricksHref() != null) { tipsAndTricksAction = IDEActionFactory.TIPS_AND_TRICKS.create(getWindow()); registerGlobalAction (tipsAndTricksAction); ps.setValue (IDEActionFactory.TIPS_AND_TRICKS.getId(), true); break; } } }
Created attachment 20471 [details] Patch migrated to 3.1 Here is the patch modified to apply against 3.1
I am going to move this to Nick who owns this code. So much work has been done here in 3.1 I afraid of breaking something. I'll let him decide if he wants to do this for 3.1.
Dan, did you determine that getFeatureInfos() is no longer getting called with this patch? It only avoids the call if the action will be shown (i.e. if old-style welcome pages, or tips and tricks, were found). Since we now have the new intro story, there should no longer be any old-style welcome pages, so it should end up looking for them each time. Or do they still exist in RAD but are not shown? A better approach would be to remember "yes/no/unknown" for both of the actions.
Nick, please bare with me. I no longer work on RAD and this defect was opened over seven months ago, so I'm struggling to remember details. << Dan, did you determine that getFeatureInfos() is no longer getting called with this patch? >> Yes. << It only avoids the call if the action will be shown (i.e. if old-style welcome pages, or tips and tricks, were found)... Since we now have the new intro story, there should no longer be any old-style welcome pages, so it should end up looking for them each time. >> The result of the prior search is YES (at least one was found) or NO (none were found). The lack of an entry is "unknown" (i.e., you have to go look). IIRC, the case for RAD was always NO, so it took the first branch of the IF and avoided the call to getFeatureInfos().
Sorry, I meant to say the answer was always yes (ps.getBoolean (IDEActionFactory.QUICK_START.getId()) returns true), i.e., RAD is using the "new" welcome / tips and tricks.
IDEActionFactory.QUICK_START is for the old welcome pages, not the new intro support. But anyway, we should cache the result in both cases, and invalidate the cache when the registry changes. I'm thinking of something like the following: AboutInfo[] infos = null; IPreferenceStore prefs = IDEWorkbenchPlugin.getDefault().getPreferenceStore(); // See if a welcome page is specified. // Optimization: if welcome pages were found on a previous run, then just add the action. String quickStartKey = IDEActionFactory.QUICK_START.getId(); String showQuickStart = prefs.getString(quickStartKey); if ("true".equals(showQuickStart)) { //$NON-NLS-1$ quickStartAction = IDEActionFactory.QUICK_START.create(window); register(quickStartAction); } else if ("false".equals(showQuickStart)) { //$NON-NLS-1$ // do nothing } else { // do the work for (int i = 0; i < infos.length; i++) { if (infos == null) { infos = IDEWorkbenchPlugin.getDefault().getFeatureInfos(); } if (infos[i].getWelcomePageURL() != null) { quickStartAction = IDEActionFactory.QUICK_START.create(window); register(quickStartAction); prefs.setValue(quickStartKey, "true"); //$NON-NLS-1$ break; } prefs.setValue(quickStartKey, "false"); //$NON-NLS-1$ } }
A variation of the patch has been released. It remembers both "show action" and "don't show action" states, and invalidates it when the platform state changes. The performance improvement in the SDK is negligible (~20ms), but it could make a difference on larger systems.
Verified in I20050512-1200.
Created attachment 164820 [details] fix The patch that has been released is incorrect: In case the quick start is not shown, this fact is not persisted in the preferences, and the optimization does not work. This is because 'IPreferenceStore.setValue(quickstartKey, false)' clears the preference setting because the default value is false. I observe the wrong behavior with Eclipse 3.6M6.
We have applied Markus' patch on our product (based on eclipse 3.6m6), and we observe an improvement of 2 seconds startup time on cold start right after reboot. We have also seen a hotspot around this in jprofiler and verified that Markus' patch fixes the issue. I'm re-opening the bug because the original fix was apparently not good enough.
I figured that rather than re-opening this bug, it is better to file a new enhancement request since the original fix did work sometimes but not always. Tracking the new, additional improvement now as bug 309837.
