Community
Participate
Working Groups
N20090412 Profiling startup of a simple headless application, there is a significant hit in the ds activator due to computation of localized manifest data. It looks like we can make an optimization similar to bug 271621 to avoid this computatation (use getHeaders("") instead of getHeaders(). The two invocations of getHeaders() in ds activation cost 8% of the total startup time. See Activator.start line 157 and SCRManager.startedBundle line 477.
Created attachment 131714 [details] Potential fix
Created attachment 131715 [details] Profiler data
Stoyan, I thought the DS implementation did some amount of caching to avoid re-parsing the component XML files on a restart. Seems like you should also cache the service component header.
To be clear, this test was on second or third restart so the framework itself was reading state entirely from caches. I think this is why it shows up as such a big hit, because everything else has been cached at this point.
(In reply to comment #3) > Stoyan, I thought the DS implementation did some amount of caching to avoid > re-parsing the component XML files on a restart. Seems like you should also > cache the service component header. DS bundle does have cache for the parsed component descriptions. It is not active by default. I will have to change that. BTW I will have to improve the cache implementation because it is not appropriate in case of heavy load and lots of components. I could cache the component header but isn't it faster if I call Bundle.getHeaders("").get(...) to access it? Are there cases in which the bundle's manifest is not loaded and therefore this solution will force its loading? The proposed by John solution improves the performance because it skips the localization of the manifest. The component header does not need localization. I have to make the same fix at several other places as well.
(In reply to comment #5) > (In reply to comment #3) > > Stoyan, I thought the DS implementation did some amount of caching to avoid > > re-parsing the component XML files on a restart. Seems like you should also > > cache the service component header. > > DS bundle does have cache for the parsed component descriptions. It is not > active by default. I will have to change that. BTW I will have to improve the > cache implementation because it is not appropriate in case of heavy load and > lots of components. > > I could cache the component header but isn't it faster if I call > Bundle.getHeaders("").get(...) to access it? Are there cases in which the > bundle's manifest is not loaded and therefore this solution will force its > loading? > > The proposed by John solution improves the performance because it skips the > localization of the manifest. The component header does not need localization. > I have to make the same fix at several other places as well. > In the framework we have something called a cached manifest. This holds a few important headers (Bundle-SymbolicName, Bundle-Version etc.). On a restart we try to avoid getting headers that are not available in the cached manifest. We also want to avoid adding any more values to the cached manifest because that is more memory needed for each bundle. Right now for the SDK we do not get any headers on a restart that fall out of the cached manifest values. This allows us to avoid loading and parsing the bundle manifests. If DS now gets the composite header for each bundle we now have to load and parse all bundle manifests on every startup. For 3.5 I would like to at least avoid doing that. It should not be that hard for the DS impl to cache the fact that a particular bundle has a component header or not. Then on restart you can avoid calling the getHeaders("") method for bundle you know do not have the header.
(In reply to comment #6) > For 3.5 I would like to at least avoid doing that. It should not be that hard > for the DS impl to cache the fact that a particular bundle has a component > header or not. Then on restart you can avoid calling the getHeaders("") method > for bundle you know do not have the header. > I'm going to take a quick stab at doing this. Stoyan, I was thinking of adding a new method to CompositeStorage to get the composite header.
(In reply to comment #7) > I'm going to take a quick stab at doing this. Stoyan, I was thinking of adding > a new method to CompositeStorage to get the composite header. > I talked with Stoyan about this a bit. Stoyan indicated that he is working on fixing a few caching bugs in DS, but there is an overall issue with how to handle the Service-Component header. It would make the DS implementation of the caching mechanism easier if the framework either made it fast to get the value of the Service-Component header (similar to how it caches other well-known headers such as Bundle-SymbolicName) or if the framework provided a low-level mechanism for testing if a bundle's manifest contained a particular header. At the Equinox call today we decided to simply add Service-Component header as one of the special headers we cache for 3.5. In a future (3.6) release we should look at other options for either extending the headers which are cached or providing some other quick way for testing for the existence of headers. Stoyan, please replace all calls to Bundle.getHeaders() with a call to Bundle.getHeaders(""). We will make the call to Bundle.getHeaders("").get("Service-Component") fast in in the framework for 3.5. Also, look at improving your caching mechanism for the parsed XML, but I think that should take a lower priority to the new DS spec implementation work.
Created attachment 131854 [details] framework patch This patch caches the Service-Component header. I released this patch to HEAD to get additional testing done on it.
Looks good. I was actually wondering if it would make sense to put in an OSGi API request for obtaining a single manifest header (obviously not helpful for 3.5 but possibly for future cases). Let me know when the ds fix is in, and I'll re-run the test scenario with the profiler again in the first nightly build containing the fix.
(In reply to comment #10) > Looks good. I was actually wondering if it would make sense to put in an OSGi > API request for obtaining a single manifest header (obviously not helpful for > 3.5 but possibly for future cases). > I opened bug 272302 to track this in a future release.
The fix for this problem is released in HEAD. Also, the caching of the parsed components is enabled by default.
Hi, I'd like to request this fix be backported to 3.4.2. Profiling org.eclipse.equinox.internal.ds.Activator.start(BundleContext), we are measuring 0.124 seconds in 3.6 versus 12 seconds in 3.4.2 We profiled this while debugging a significant startup delay with a customer.
Hi, I wanted to see if there's an outlook for comment 13? Also, do you prefer a separate bugzilla or is continuing the discussion here preferred? Thanks Billy
(In reply to comment #14) > Hi, > > I wanted to see if there's an outlook for comment 13? > Also, do you prefer a separate bugzilla or is continuing the discussion here > preferred? > > Thanks > Billy There is no planned release of 3.4.x. There is significant code changes in 3.5 that would make this a rather difficult thing to backport. Stoyan, what are your thoughts on this? Do you know what code changes were required to fix this?
(In reply to comment #15) > (In reply to comment #14) > > Hi, > > > > I wanted to see if there's an outlook for comment 13? > > Also, do you prefer a separate bugzilla or is continuing the discussion here > > preferred? > > > > Thanks > > Billy > > There is no planned release of 3.4.x. There is significant code changes in 3.5 > that would make this a rather difficult thing to backport. Stoyan, what are > your thoughts on this? Do you know what code changes were required to fix > this? I think I can prepare a patch for DS for Equinox 3.4.2.
Created attachment 178960 [details] patch for DS bundle released in equinox 3.4.2 I am attaching a patch for DS bundle released with equinox 3.4.2 (DS bundle is tagged with tag "R3_4_2") The patch only replaces the expensive calls to Bundle.getHeaders() with Bundle.getHeaders(""). Please note that you will not probably observe the same speed as in equinox 3.6, because part of the optimization is located in the framework.
Thanks for the patch. It appears to be a keeper! Here's our profiling information (numbers are for relative comparison only) Thanks to Kevan Holdaway for testing the patch and collecting the data! On the current 3.4.2 stream: Without patch: 2687 ms With patch: 761 ms Earlier 3.4x stream (in our released product) Without patch: 12,134ms With patch: 2,703ms In 3.6, the code is in the 0.1 second range. Thanks for the fast turnaround! Billy
Thanks again for the patch. Could you let us know when this is released to a 3.4x stream?
