Attachment 338850 Details for Bug 184904 – patch for landing

[patch] patch for landing

b-backup.diff (text/plain), 4.19 KB, created by Saam Barati on 2018-04-25 19:49:11 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-04-25 19:49:11 PDT

Size: 4.19 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 231039)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,26 @@
>+2018-04-25  Saam Barati  <sbarati@apple.com>
>+
>+        dlopen the bundle's executable before calling -[NSBundle load] since that will also do a bunch of other things we don't need
>+        https://bugs.webkit.org/show_bug.cgi?id=184904
>+
>+        Reviewed by Geoffrey Garen.
>+
>+        Loading an NSBundle does a lot of work to find the principal class inside
>+        the bundle. This means it walks all the objective C class names loaded
>+        by the bundle. Doing this is *really* expensive.
>+        
>+        Some users of the injected bundle define a WKBundleInitialize function.
>+        In such a case, we don't need the principal class, so we can skip loading
>+        the NSBundle. Now, before we load the bundle, we dlopen and dlsym looking
>+        for the WKBundleInitialize function. If we find it, we skip loading
>+        the bundle. If we don't find the WKBundleInitialize function, we fall
>+        back to loading the bundle and finding the principal class.
>+        
>+        This speeds up initializeWebProcess by ~70ms on my MBP.
>+
>+        * WebProcess/InjectedBundle/mac/InjectedBundleMac.mm:
>+        (WebKit::InjectedBundle::initialize):
>+
> 2018-04-25  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         PSON: Don't create a new process when navigating to a blob URL, data URL, and about:blank
>Index: Source/WebKit/WebProcess/InjectedBundle/mac/InjectedBundleMac.mm
>===================================================================
>--- Source/WebKit/WebProcess/InjectedBundle/mac/InjectedBundleMac.mm	(revision 231037)
>+++ Source/WebKit/WebProcess/InjectedBundle/mac/InjectedBundleMac.mm	(working copy)
>@@ -33,7 +33,9 @@
> #import "WKWebProcessBundleParameters.h"
> #import "WKWebProcessPlugInInternal.h"
> #import "WebProcessCreationParameters.h"
>+#import <CoreFoundation/CFURL.h>
> #import <Foundation/NSBundle.h>
>+#import <dlfcn.h>
> #import <pal/spi/cocoa/NSKeyedArchiverSPI.h>
> #import <stdio.h>
> #import <wtf/RetainPtr.h>
>@@ -76,10 +78,23 @@ bool InjectedBundle::initialize(const We
>         WTFLogAlways("InjectedBundle::load failed - Could not create the bundle.\n");
>         return false;
>     }
>+
>+    WKBundleInitializeFunctionPtr initializeFunction = nullptr;
>+    if (RetainPtr<CFURLRef> executableURL = adoptCF(CFBundleCopyExecutableURL([m_platformBundle _cfBundle]))) {
>+        static constexpr size_t maxPathSize = 4096;
>+        char pathToExecutable[maxPathSize];
>+        if (CFURLGetFileSystemRepresentation(executableURL.get(), true, bitwise_cast<uint8_t*>(pathToExecutable), maxPathSize)) {
>+            // We don't hold onto this handle anywhere more permanent since we never dlcose.
>+            if (void* handle = dlopen(pathToExecutable, RTLD_LAZY | RTLD_GLOBAL | RTLD_FIRST))
>+                initializeFunction = bitwise_cast<WKBundleInitializeFunctionPtr>(dlsym(handle, "WKBundleInitialize"));
>+        }
>+    }
>         
>-    if (![m_platformBundle load]) {
>-        WTFLogAlways("InjectedBundle::load failed - Could not load the executable from the bundle.\n");
>-        return false;
>+    if (!initializeFunction) {
>+        if (![m_platformBundle load]) {
>+            WTFLogAlways("InjectedBundle::load failed - Could not load the executable from the bundle.\n");
>+            return false;
>+        }
>     }
> 
> #if WK_API_ENABLED
>@@ -100,9 +115,11 @@ bool InjectedBundle::initialize(const We
>         m_bundleParameters = adoptNS([[WKWebProcessBundleParameters alloc] initWithDictionary:dictionary]);
>     }
> #endif
>+    
>+    if (!initializeFunction)
>+        initializeFunction = bitwise_cast<WKBundleInitializeFunctionPtr>(CFBundleGetFunctionPointerForName([m_platformBundle _cfBundle], CFSTR("WKBundleInitialize")));
> 
>     // First check to see if the bundle has a WKBundleInitialize function.
>-    WKBundleInitializeFunctionPtr initializeFunction = reinterpret_cast<WKBundleInitializeFunctionPtr>(CFBundleGetFunctionPointerForName([m_platformBundle _cfBundle], CFSTR("WKBundleInitialize")));
>     if (initializeFunction) {
>         initializeFunction(toAPI(this), toAPI(initializationUserData));
>         return true;

Actions: View | Formatted Diff | Diff

Attachments on bug 184904: 338627 | 338692 | 338850