Fixed a performance regression that was caused by uncached access to the
bundle manifest headers. (FELIX-711)


git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@692559 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
index 74574f1..f996c01 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
@@ -330,7 +330,7 @@
 
     public String getSymbolicName()
     {
-        return m_felix.getBundleSymbolicName(this);
+        return m_info.getSymbolicName();
     }
 
     public boolean hasPermission(Object obj)
@@ -427,10 +427,10 @@
 
     public String toString()
     {
-        String sym = m_felix.getBundleSymbolicName(this);
+        String sym = m_info.getSymbolicName();
         if (sym != null)
         {
-            return m_felix.getBundleSymbolicName(this) + " [" + getBundleId() +"]";
+            return sym + " [" + getBundleId() +"]";
         }
         return "[" + getBundleId() +"]";
     }
diff --git a/framework/src/main/java/org/apache/felix/framework/BundleInfo.java b/framework/src/main/java/org/apache/felix/framework/BundleInfo.java
index 214e9a7..fdb5fb2 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleInfo.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleInfo.java
@@ -24,6 +24,9 @@
 import java.util.*;
 
 import org.apache.felix.framework.cache.BundleArchive;
+import org.apache.felix.framework.searchpolicy.ModuleDefinition;
+import org.apache.felix.framework.util.manifestparser.ManifestParser;
+import org.apache.felix.moduleloader.ICapability;
 import org.apache.felix.moduleloader.IContentLoader;
 import org.apache.felix.moduleloader.IModule;
 import org.osgi.framework.*;
@@ -36,9 +39,11 @@
     private int m_state = 0;
     private BundleActivator m_activator = null;
     private BundleContext m_context = null;
+    private String m_cachedSymbolicName = null;
+    private long m_cachedSymbolicNameTimestamp;
     private Map m_cachedHeaders = new HashMap();
     private long m_cachedHeadersTimestamp;
-    
+
     // Indicates whether the bundle is stale, meaning that it has
     // been refreshed and completely removed from the framework.
     private boolean m_stale = false;
@@ -131,6 +136,31 @@
         m_modules = dest;
     }
 
+    public synchronized String getSymbolicName()
+    {
+        // If the bundle has been updated, clear the cached symbolic name.
+        if (getLastModified() > m_cachedSymbolicNameTimestamp)
+        {
+            m_cachedSymbolicName = null;
+            m_cachedSymbolicNameTimestamp = getLastModified();
+            try
+            {
+                // TODO: FRAMEWORK - Rather than reparsing every time, I wonder if
+                //       we should be caching this value some place.
+                final ICapability moduleCap = ManifestParser.parseBundleSymbolicName(getCurrentHeader());
+                if (moduleCap != null)
+                {
+                    m_cachedSymbolicName = (String) moduleCap.getProperties().get(Constants.BUNDLE_SYMBOLICNAME_ATTRIBUTE);
+                }
+            }
+            catch (BundleException ex)
+            {
+                // Return null.
+            }
+        }
+        return m_cachedSymbolicName;
+    }
+
     public long getBundleId()
     {
         try
@@ -196,20 +226,27 @@
 
     public Map getCurrentHeader()
     {
-        try
+        Map headerMap = null;
+        // Special case the system bundle
+        if (getBundleId() == 0)
         {
-            // Return the header for the most recent bundle revision only,
-            // since we shouldn't ever need access to older revisions.
-            return m_archive.getRevision(m_archive.getRevisionCount() - 1).getManifestHeader();
+            // TODO: REFACTOR - This is sort of a hack, we should just expose
+            //       the bundle symbolic name from our API.
+            try
+            {
+                headerMap = m_archive.getRevision(0).getManifestHeader();
+            }
+            catch (Exception ex)
+            {
+                // This should never happen.
+            }
         }
-        catch (Exception ex)
+        else
         {
-            m_logger.log(
-                Logger.LOG_ERROR,
-                "Error reading manifest from bundle archive.",
-                ex);
-            return null;
+            headerMap = ((ModuleDefinition) getCurrentModule().getDefinition()).getHeaders();
         }
+            
+        return headerMap;
     }
 
     public Map getCurrentLocalizedHeader(String locale)
@@ -231,21 +268,9 @@
             }
         }
 
-        Map headers;
-        try
-        {
-            Map rawHeaders = m_archive.getRevision(m_archive.getRevisionCount() - 1).getManifestHeader();
-            headers = new HashMap(rawHeaders.size());
-            headers.putAll(rawHeaders);
-        }
-        catch (Exception ex)
-        {
-            m_logger.log(
-                Logger.LOG_ERROR,
-                "Error reading manifest from bundle archive.",
-                ex);
-            return null;
-        }
+        Map rawHeaders = getCurrentHeader();
+        Map headers = new HashMap(rawHeaders.size());
+        headers.putAll(rawHeaders);
 
         // Check to see if we actually need to localize anything
         boolean needsLocalization = false;
@@ -321,7 +346,7 @@
 
     private void updateHeaderCache(String locale, Map localizedHeaders)
     {
-        synchronized(m_cachedHeaders)
+        synchronized (m_cachedHeaders)
         {
             m_cachedHeaders.put(locale, localizedHeaders);
             m_cachedHeadersTimestamp = System.currentTimeMillis();
@@ -533,22 +558,22 @@
         m_lockCount = info.m_lockCount;
         m_lockThread = info.m_lockThread;
     }
-    
+
     public synchronized void setProtectionDomain(ProtectionDomain pd)
     {
         getCurrentModule().getContentLoader().setSecurityContext(pd);
     }
-    
+
     public synchronized ProtectionDomain getProtectionDomain()
     {
         ProtectionDomain pd = null;
-        
+
         for (int i = m_modules.length - 1; (i >= 0) && (pd == null); i--)
         {
-            pd = (ProtectionDomain) 
+            pd = (ProtectionDomain)
                 m_modules[i].getContentLoader().getSecurityContext();
         }
-        
+
         return pd;
     }
 }
\ No newline at end of file
diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java b/framework/src/main/java/org/apache/felix/framework/Felix.java
index 70616c6..461d167 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -1334,28 +1334,6 @@
     //
 
     /**
-     * Implementation for Bundle.getSymbolicName().
-    **/
-    protected String getBundleSymbolicName(FelixBundle bundle)
-    {
-        try
-        {
-            // TODO: FRAMEWORK - Rather than reparsing every time, I wonder if
-            //       we should be caching this value some place.
-            final ICapability moduleCap = ManifestParser.parseBundleSymbolicName(bundle.getInfo().getCurrentHeader());
-            if (moduleCap != null)
-            {
-                return (String) moduleCap.getProperties().get(Constants.BUNDLE_SYMBOLICNAME_ATTRIBUTE);
-            }
-        }
-        catch (BundleException ex)
-        {
-            // Return null.
-        }
-        return null;
-    }
-
-    /**
      * Get bundle headers and resolve any localized strings from resource bundles.
      * @param bundle
      * @param locale
@@ -1828,13 +1806,15 @@
                     // extension bundle (info.isExtension) or an update from
                     // a normal bundle to an extension bundle
                     // (isExtensionBundle())
+                    Map headerMap = archive.getRevision(
+                        archive.getRevisionCount() - 1).getManifestHeader();
                     IModule module = createModule(
                         info.getBundleId(),
                         archive.getRevisionCount() - 1,
-                        info.getCurrentHeader(),
+                        headerMap,
                         (bundle.getInfo().isExtension() ||
                         m_extensionManager.isExtensionBundle(
-                            bundle.getInfo().getCurrentHeader())));
+                            headerMap)));
 
                     // Add module to bundle info.
                     info.addModule(module);
@@ -2330,9 +2310,11 @@
             try
             {
                 BundleArchive archive = m_cache.getArchive(id);
-                bundle = new BundleImpl(this, createBundleInfo(archive,
-                    m_extensionManager.isExtensionBundle(archive.getRevision(
-                    archive.getRevisionCount() - 1).getManifestHeader())));
+                Map headerMap = archive.getRevision(
+                    archive.getRevisionCount() - 1).getManifestHeader();
+                bundle = new BundleImpl(
+                    this, createBundleInfo(
+                        archive, headerMap, m_extensionManager.isExtensionBundle(headerMap)));
 
                 verifyExecutionEnvironment(bundle);
 
@@ -3325,28 +3307,9 @@
     // Miscellaneous private methods.
     //
 
-    private BundleInfo createBundleInfo(BundleArchive archive, boolean isExtension)
+    private BundleInfo createBundleInfo(BundleArchive archive, Map headerMap, boolean isExtension)
         throws Exception
     {
-        // Get the bundle manifest.
-        Map headerMap = null;
-        try
-        {
-            // Although there should only ever be one revision at this
-            // point, get the header for the current revision to be safe.
-            headerMap = archive.getRevision(archive.getRevisionCount() - 1).getManifestHeader();
-        }
-        catch (Exception ex)
-        {
-            throw new BundleException("Unable to read JAR manifest.", ex);
-        }
-
-        // We can't do anything without the manifest header.
-        if (headerMap == null)
-        {
-            throw new BundleException("Unable to read JAR manifest header.");
-        }
-
         // Create the module for the bundle; although there should only
         // ever be one revision at this point, create the module for
         // the current revision to be safe.
@@ -3509,9 +3472,7 @@
         {
             try
             {
-                activator =
-                    m_cache.getArchive(info.getBundleId())
-                        .getActivator(info.getCurrentModule());
+                activator = info.getArchive().getActivator(info.getCurrentModule());
             }
             catch (Exception ex)
             {
@@ -3523,13 +3484,8 @@
         // class from the bundle manifest.
         if (activator == null)
         {
-            // Get the associated bundle archive.
-            BundleArchive ba = m_cache.getArchive(info.getBundleId());
-            // Get the manifest from the current revision; revision is
-            // base zero so subtract one from the count to get the
-            // current revision.
-            Map headerMap = ba.getRevision(ba.getRevisionCount() - 1).getManifestHeader();
-            // Get the activator class attribute.
+            // Get the activator class from the header map.
+            Map headerMap = info.getCurrentHeader();
             String className = (String) headerMap.get(Constants.BUNDLE_ACTIVATOR);
             // Try to instantiate activator class if present.
             if (className != null)
@@ -4046,8 +4002,8 @@
                 try
                 {
                     BundleInfo info = m_bundle.getInfo();
-                    BundleInfo newInfo = createBundleInfo(info.getArchive(),
-                        info.isExtension());
+                    BundleInfo newInfo = createBundleInfo(
+                        info.getArchive(), info.getCurrentHeader(), info.isExtension());
                     newInfo.syncLock(info);
                     ((BundleImpl) m_bundle).setInfo(newInfo);
                     addSecurity(m_bundle);