Bug 1529894 - Change jar log content. r=aklotz,chmanchester
☠☠ backed out by 24e79b0a187f ☠ ☠
authorMike Hommey <mh+mozilla@glandium.org>
Wed, 06 Mar 2019 01:18:10 +0000
changeset 520431 3c37fd9d30d50b66b9b899c3f85758079f8e31d1
parent 520430 e93f4871731b8e658c7febf013cd985738ad1435
child 520432 e2d7b59776a26e24783e4eb07665e0f0cdd46aaf
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaklotz, chmanchester
bugs1529894
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1529894 - Change jar log content. r=aklotz,chmanchester The jar log is used for optimization of the packaged jar files according to their usage patterns during a profile run. The current content of the file currently come with 2 caveats: - it contains entries for jar archives that aren't relevant to packaging, which is not a problem in itself, but see below. - it contains full paths for jar archives that may not correspond to the location of the packaged directory (on e.g. Android, where the build almost certainly doesn't happen in the same directory on the host as Fennec runs in the emulator/on the device). The current JarLog code does somehow handle the various ways paths are currently presented, but it's clearly missing code to map the paths in the log to packaged paths. Instead of requiring manual work and extra build options to handle this mapping, and considering the caveats above, it's just simpler to log archive paths as if they were relative to the packaged application directory in a build, and use that during packaging. Depends on D21655 Differential Revision: https://phabricator.services.mozilla.com/D21656
modules/libjar/nsZipArchive.cpp
python/mozbuild/mozpack/mozjar.py
python/mozbuild/mozpack/test/test_mozjar.py
toolkit/mozapps/installer/packager.py
--- a/modules/libjar/nsZipArchive.cpp
+++ b/modules/libjar/nsZipArchive.cpp
@@ -17,16 +17,17 @@
 #endif
 #include "nsISupportsUtils.h"
 #include "prio.h"
 #include "plstr.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Logging.h"
 #include "mozilla/UniquePtrExtensions.h"
 #include "stdlib.h"
+#include "nsDirectoryService.h"
 #include "nsWildCard.h"
 #include "nsXULAppAPI.h"
 #include "nsZipArchive.h"
 #include "nsString.h"
 #include "prenv.h"
 #if defined(XP_WIN)
 #  include <windows.h>
 #endif
@@ -334,17 +335,56 @@ nsresult nsZipArchive::OpenArchive(nsZip
 
   //-- get table of contents for archive
   nsresult rv = BuildFileList(aFd);
   if (NS_SUCCEEDED(rv)) {
     if (aZipHandle->mFile && XRE_IsParentProcess()) {
       static char *env = PR_GetEnv("MOZ_JAR_LOG_FILE");
       if (env) {
         zipLog.Init(env);
-        aZipHandle->mFile.GetURIString(mURI);
+        // We only log accesses in jar/zip archives within the NS_GRE_DIR
+        // and/or the APK on Android. For the former, we log the archive path
+        // relative to NS_GRE_DIR, and for the latter, the nested-archive
+        // path within the APK. This makes the path match the path of the
+        // archives relative to the packaged dist/$APP_NAME directory in a
+        // build.
+        if (aZipHandle->mFile.IsZip()) {
+          // Nested archive, likely omni.ja in APK.
+          aZipHandle->mFile.GetPath(mURI);
+        } else if (nsDirectoryService::gService) {
+          // We can reach here through the initialization of Omnijar from
+          // XRE_InitCommandLine, which happens before the directory service
+          // is initialized. When that happens, it means the opened archive is
+          // the APK, and we don't care to log that one, so we just skip
+          // when the directory service is not initialized.
+          nsCOMPtr<nsIFile> dir = aZipHandle->mFile.GetBaseFile();
+          nsCOMPtr<nsIFile> gre_dir;
+          nsAutoCString path;
+          if (NS_SUCCEEDED(nsDirectoryService::gService->Get(
+                  NS_GRE_DIR, NS_GET_IID(nsIFile), getter_AddRefs(gre_dir)))) {
+            nsAutoCString leaf;
+            nsCOMPtr<nsIFile> parent;
+            while (NS_SUCCEEDED(dir->GetNativeLeafName(leaf)) &&
+                   NS_SUCCEEDED(dir->GetParent(getter_AddRefs(parent)))) {
+              if (!parent) {
+                break;
+              }
+              dir = parent;
+              if (path.Length()) {
+                path.Insert('/', 0);
+              }
+              path.Insert(leaf, 0);
+              bool equals;
+              if (NS_SUCCEEDED(dir->Equals(gre_dir, &equals)) && equals) {
+                mURI.Assign(path);
+                break;
+              }
+            }
+          }
+        }
       }
     }
   }
   return rv;
 }
 
 nsresult nsZipArchive::OpenArchive(nsIFile *aFile) {
   RefPtr<nsZipHandle> handle;
--- a/python/mozbuild/mozpack/mozjar.py
+++ b/python/mozbuild/mozpack/mozjar.py
@@ -828,61 +828,23 @@ class BrotliCompress(object):
 
     def flush(self):
         return Brotli.compress(self._buf.getvalue())
 
 
 class JarLog(dict):
     '''
     Helper to read the file Gecko generates when setting MOZ_JAR_LOG_FILE.
-    The jar log is then available as a dict with the jar path as key (see
-    canonicalize for more details on the key value), and the corresponding
-    access log as a list value. Only the first access to a given member of
-    a jar is stored.
+    The jar log is then available as a dict with the jar path as key, and
+    the corresponding access log as a list value. Only the first access to
+    a given member of a jar is stored.
     '''
 
     def __init__(self, file=None, fileobj=None):
         if not fileobj:
             fileobj = open(file, 'r')
-        urlmap = {}
         for line in fileobj:
-            url, path = line.strip().split(None, 1)
-            if not url or not path:
+            jar, path = line.strip().split(None, 1)
+            if not jar or not path:
                 continue
-            if url not in urlmap:
-                urlmap[url] = JarLog.canonicalize(url)
-            jar = urlmap[url]
             entry = self.setdefault(jar, [])
             if path not in entry:
                 entry.append(path)
-
-    @staticmethod
-    def canonicalize(url):
-        '''
-        The jar path is stored in a MOZ_JAR_LOG_FILE log as a url. This method
-        returns a unique value corresponding to such urls.
-        - file:///{path} becomes {path}
-        - jar:file:///{path}!/{subpath} becomes ({path}, {subpath})
-        - jar:jar:file:///{path}!/{subpath}!/{subpath2} becomes
-           ({path}, {subpath}, {subpath2})
-        '''
-        if not isinstance(url, ParseResult):
-            # Assume that if it doesn't start with jar: or file:, it's a path.
-            if not url.startswith(('jar:', 'file:')):
-                url = 'file:///' + os.path.abspath(url)
-            url = urlparse(url)
-        assert url.scheme
-        assert url.scheme in ('jar', 'file')
-        if url.scheme == 'jar':
-            path = JarLog.canonicalize(url.path)
-            if isinstance(path, tuple):
-                return path[:-1] + tuple(path[-1].split('!/', 1))
-            return tuple(path.split('!/', 1))
-        if url.scheme == 'file':
-            assert os.path.isabs(url.path)
-            path = url.path
-            # On Windows, url.path will be /drive:/path ; on Unix systems,
-            # /path. As we want drive:/path instead of /drive:/path on Windows,
-            # remove the leading /.
-            if os.path.isabs(path[1:]):
-                path = path[1:]
-            path = os.path.realpath(path)
-            return mozpath.normsep(os.path.normcase(path))
--- a/python/mozbuild/mozpack/test/test_mozjar.py
+++ b/python/mozbuild/mozpack/test/test_mozjar.py
@@ -285,61 +285,38 @@ class TestPreload(unittest.TestCase):
 
         self.assertEqual(files[0].filename, 'baz/qux')
         self.assertEqual(files[1].filename, 'bar')
         self.assertEqual(files[2].filename, 'foo')
 
 
 class TestJarLog(unittest.TestCase):
     def test_jarlog(self):
-        base = 'file:' + pathname2url(os.path.abspath(os.curdir))
         s = StringIO('\n'.join([
-            base + '/bar/baz.jar first',
-            base + '/bar/baz.jar second',
-            base + '/bar/baz.jar third',
-            base + '/bar/baz.jar second',
-            base + '/bar/baz.jar second',
-            'jar:' + base + '/qux.zip!/omni.ja stuff',
-            base + '/bar/baz.jar first',
-            'jar:' + base + '/qux.zip!/omni.ja other/stuff',
-            'jar:' + base + '/qux.zip!/omni.ja stuff',
-            base + '/bar/baz.jar third',
-            'jar:jar:' + base + '/qux.zip!/baz/baz.jar!/omni.ja nested/stuff',
-            'jar:jar:jar:' + base + '/qux.zip!/baz/baz.jar!/foo.zip!/omni.ja' +
-            ' deeply/nested/stuff',
+            'bar/baz.jar first',
+            'bar/baz.jar second',
+            'bar/baz.jar third',
+            'bar/baz.jar second',
+            'bar/baz.jar second',
+            'omni.ja stuff',
+            'bar/baz.jar first',
+            'omni.ja other/stuff',
+            'omni.ja stuff',
+            'bar/baz.jar third',
         ]))
         log = JarLog(fileobj=s)
-
-        def canonicalize(p):
-            return mozpath.normsep(os.path.normcase(os.path.realpath(p)))
-
-        baz_jar = canonicalize('bar/baz.jar')
-        qux_zip = canonicalize('qux.zip')
         self.assertEqual(set(log.keys()), set([
-            baz_jar,
-            (qux_zip, 'omni.ja'),
-            (qux_zip, 'baz/baz.jar', 'omni.ja'),
-            (qux_zip, 'baz/baz.jar', 'foo.zip', 'omni.ja'),
+            'bar/baz.jar',
+            'omni.ja',
         ]))
-        self.assertEqual(log[baz_jar], [
+        self.assertEqual(log['bar/baz.jar'], [
             'first',
             'second',
             'third',
         ])
-        self.assertEqual(log[(qux_zip, 'omni.ja')], [
+        self.assertEqual(log['omni.ja'], [
             'stuff',
             'other/stuff',
         ])
-        self.assertEqual(log[(qux_zip, 'baz/baz.jar', 'omni.ja')],
-                         ['nested/stuff'])
-        self.assertEqual(log[(qux_zip, 'baz/baz.jar', 'foo.zip',
-                              'omni.ja')], ['deeply/nested/stuff'])
-
-        # The above tests also indirectly check the value returned by
-        # JarLog.canonicalize for various jar: and file: urls, but
-        # JarLog.canonicalize also supports plain paths.
-        self.assertEqual(JarLog.canonicalize(os.path.abspath('bar/baz.jar')),
-                         baz_jar)
-        self.assertEqual(JarLog.canonicalize('bar/baz.jar'), baz_jar)
 
 
 if __name__ == '__main__':
     mozunit.main()
--- a/toolkit/mozapps/installer/packager.py
+++ b/toolkit/mozapps/installer/packager.py
@@ -319,19 +319,18 @@ def main():
 
     # Setup preloading
     if args.jarlog and os.path.exists(args.jarlog):
         from mozpack.mozjar import JarLog
         log = JarLog(args.jarlog)
         for p, f in copier:
             if not isinstance(f, Jarrer):
                 continue
-            key = JarLog.canonicalize(os.path.join(args.destination, p))
-            if key in log:
-                f.preload(log[key])
+            if p in log:
+                f.preload(log[p])
 
     copier.copy(args.destination)
     generate_precomplete(os.path.normpath(os.path.join(args.destination,
                                                        respath)))
 
 
 if __name__ == '__main__':
     main()