Reviewed by Darin.
authormjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 May 2004 23:20:12 +0000 (23:20 +0000)
committermjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 May 2004 23:20:12 +0000 (23:20 +0000)
- -[WebFrame childFrames] is so hot that a special internal
version which avoids the copy and autorelease results in a .75%
performance improvement on HTML iBench.

        * WebView.subproj/WebFramePrivate.h: Prototype new method.
        * WebView.subproj/WebFrame.m:
(-[WebFrame _internalChildFrames]): New method, just returns
internal value instead of copying.

        (-[WebFrame _descendantFrameNamed:]): Use it
        (-[WebFrame _textSizeMultiplierChanged]): likewise
        (-[WebFrame _viewWillMoveToHostWindow:]): likewise
        (-[WebFrame _viewDidMoveToHostWindow]): likewise
        (-[WebFrame _saveDocumentAndScrollState]): likewise
        (-[WebFrame _numPendingOrLoadingRequests:]): likewise
        (-[WebFrame _checkLoadComplete]): Refactored this and it's two
helpers a little so we could get away with using
_internalChildFrames.
        (-[WebFrame _checkLoadCompleteForThisFrame]): Renamed from
_isLoadComplete
(-[WebFrame _recursiveCheckLoadComplete]): renamed from (class
method) _recursiveCheckCompleteFromFrame:
        * WebView.subproj/WebDataSource.m:
        (-[WebDataSource _defersCallbacksChanged]): Use it
        (-[WebDataSource isLoading]): likewise
        * WebView.subproj/WebView.m:
        (-[WebView _frameForDataSource:fromFrame:]): likewise
        (-[WebView _frameForView:fromFrame:]): likewise

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@6569 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKit/ChangeLog
WebKit/WebView.subproj/WebDataSource.m
WebKit/WebView.subproj/WebFrame.m
WebKit/WebView.subproj/WebFramePrivate.h
WebKit/WebView.subproj/WebView.m

index 0c45b9f..dced1ca 100644 (file)
@@ -1,3 +1,36 @@
+2004-05-08  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Darin.
+
+       - -[WebFrame childFrames] is so hot that a special internal
+       version which avoids the copy and autorelease results in a .75%
+       performance improvement on HTML iBench.
+
+        * WebView.subproj/WebFramePrivate.h: Prototype new method.
+        * WebView.subproj/WebFrame.m:
+       (-[WebFrame _internalChildFrames]): New method, just returns
+       internal value instead of copying.
+
+        (-[WebFrame _descendantFrameNamed:]): Use it
+        (-[WebFrame _textSizeMultiplierChanged]): likewise
+        (-[WebFrame _viewWillMoveToHostWindow:]): likewise
+        (-[WebFrame _viewDidMoveToHostWindow]): likewise
+        (-[WebFrame _saveDocumentAndScrollState]): likewise
+        (-[WebFrame _numPendingOrLoadingRequests:]): likewise
+        (-[WebFrame _checkLoadComplete]): Refactored this and it's two
+       helpers a little so we could get away with using
+       _internalChildFrames.
+        (-[WebFrame _checkLoadCompleteForThisFrame]): Renamed from
+       _isLoadComplete
+       (-[WebFrame _recursiveCheckLoadComplete]): renamed from (class
+       method) _recursiveCheckCompleteFromFrame:
+        * WebView.subproj/WebDataSource.m:
+        (-[WebDataSource _defersCallbacksChanged]): Use it
+        (-[WebDataSource isLoading]): likewise
+        * WebView.subproj/WebView.m:
+        (-[WebView _frameForDataSource:fromFrame:]): likewise
+        (-[WebView _frameForView:fromFrame:]): likewise
+
 2004-05-10  Chris Blumenberg  <cblu@apple.com>
 
        Forgot to commit this copied header.
index eb5781a..6cade9d 100644 (file)
         [client setDefersCallbacks:defers];
     }
 
-    [[[self webFrame] childFrames] makeObjectsPerformSelector:@selector(_defersCallbacksChanged)];
+    // It's OK to use the internal version of getting the child
+    // frames, since undeferring callbacks will not do any immediate
+    // work, and so the set of frames can't change out from under us.
+    [[[self webFrame] _internalChildFrames] makeObjectsPerformSelector:@selector(_defersCallbacksChanged)];
 }
 
 - (NSURLRequest *)_originalRequest
     // Put in the auto-release pool because it's common to call this from a run loop source,
     // and then the entire list of frames lasts until the next autorelease.
     NSAutoreleasePool *pool = [NSAutoreleasePool new];
-    NSEnumerator *e = [[[self webFrame] childFrames] objectEnumerator];
+
+    // It's OK to use the internal version of getting the child
+    // frames, since nothing we do here can possibly change the set of
+    // frames.
+    NSEnumerator *e = [[[self webFrame] _internalChildFrames] objectEnumerator];
     WebFrame *childFrame;
     while ((childFrame = [e nextObject])) {
         if ([[childFrame dataSource] isLoading] || [[childFrame provisionalDataSource] isLoading]) {
index 8334ab5..fdcbbe5 100644 (file)
@@ -100,6 +100,35 @@ NSString *WebPageCacheDocumentViewKey = @"WebPageCacheDocumentViewKey";
 
 @end
 
+@interface NSArray (WebSafeMakePerform)
+
+- (void)_web_safeMakeObjectsPerformSelector:(SEL)aSelector
+
+@end
+
+
+@implementation NSArray (WebSafeMakePerform)
+
+- (void)_web_safeMakeObjectsPerformSelector:(SEL)aSelector
+{
+    unsigned count = [self count];
+    if (0 == count)
+       return;
+
+    if (count > 128) {
+       [[self copy] makeObjectsPerformSelector:aSelector];
+       return;
+    }
+    id batch[128];
+    [self getObjects:batch range:NSMakeRange(0, count)];
+    unsigned i;
+    for (i = 0; i < count; i++) {
+       objc_msgSend(batch[i], aSelector);
+    }
+}
+
+@end
 
 // One day we might want to expand the use of this kind of class such that we'd receive one
 // over the bridge, and possibly hand it on through to the FormsDelegate.
@@ -454,7 +483,10 @@ NSString *WebPageCacheDocumentViewKey = @"WebPageCacheDocumentViewKey";
         return self;
     }
 
-    NSArray *children = [self childFrames];
+    // It's OK to use the internal version of getting the child
+    // frames, since we know this method won't change the set of
+    // frames
+    NSArray *children = [self _internalChildFrames];
     WebFrame *frame;
     unsigned i;
 
@@ -841,7 +873,7 @@ NSString *WebPageCacheDocumentViewKey = @"WebPageCacheDocumentViewKey";
 
     if (pageCache){
         [[self dataSource] _setPrimaryLoadComplete: YES];
-        [self _isLoadComplete];
+        [self _checkLoadCompleteForThisFrame];
     }
 }
 
@@ -1015,7 +1047,7 @@ static CFAbsoluteTime _timeOfLastCompletedLoad;
     }
 }
 
-- (void)_isLoadComplete
+- (void)_checkLoadCompleteForThisFrame
 {
     ASSERT([self webView] != nil);
 
@@ -1162,21 +1194,13 @@ static CFAbsoluteTime _timeOfLastCompletedLoad;
     ASSERT_NOT_REACHED();
 }
 
-+ (void)_recursiveCheckCompleteFromFrame: (WebFrame *)fromFrame
+- (void)_recursiveCheckLoadComplete
 {
-    int i, count;
-    NSArray *childFrames;
-    
-    childFrames = [fromFrame childFrames];
-    count = [childFrames count];
-    for (i = 0; i < count; i++) {
-        WebFrame *childFrame;
-        
-        childFrame = [childFrames objectAtIndex: i];
-        [WebFrame _recursiveCheckCompleteFromFrame: childFrame];
-        [childFrame _isLoadComplete];
-    }
-    [fromFrame _isLoadComplete];
+    // Checking for load complete may indeed alter the set of child
+    // frames. However, _web_safeMakeObjectsPerformSelector: makes
+    // sure to copy the array so it is safe against changes.
+    [[self _internalChildFrames] _web_safeMakeObjectsPerformSelector:@selector(_recursiveCheckLoadComplete)];
+    [self _checkLoadCompleteForThisFrame];
 }
 
 // Called every time a resource is completely loaded, or an error is received.
@@ -1185,7 +1209,7 @@ static CFAbsoluteTime _timeOfLastCompletedLoad;
     ASSERT([self webView] != nil);
 
     // Now walk the frame tree to see if any frame that may have initiated a load is done.
-    [WebFrame _recursiveCheckCompleteFromFrame: [[self webView] mainFrame]];
+    [[[self webView] mainFrame] _recursiveCheckLoadComplete];
 }
 
 - (WebBridge *)_bridge
@@ -2037,7 +2061,9 @@ static CFAbsoluteTime _timeOfLastCompletedLoad;
         [(NSView <_web_WebDocumentTextSizing> *)view _web_textSizeMultiplierChanged];
     }
 
-    [[self childFrames] makeObjectsPerformSelector:@selector(_textSizeMultiplierChanged)];
+    // It's OK to use the internal version because this method is
+    // guaranteed not to change the set of frames.
+    [[self _internalChildFrames] makeObjectsPerformSelector:@selector(_textSizeMultiplierChanged)];
 }
 
 - (void)_defersCallbacksChanged
@@ -2049,13 +2075,17 @@ static CFAbsoluteTime _timeOfLastCompletedLoad;
 - (void)_viewWillMoveToHostWindow:(NSWindow *)hostWindow
 {
     [[[self frameView] documentView] viewWillMoveToHostWindow:hostWindow];
-    [[self childFrames] makeObjectsPerformSelector:@selector(_viewWillMoveToHostWindow:) withObject:hostWindow];
+    // It's OK to use the internal version because this method is
+    // guaranteed not to change the set of frames.
+    [[self _internalChildFrames] makeObjectsPerformSelector:@selector(_viewWillMoveToHostWindow:) withObject:hostWindow];
 }
 
 - (void)_viewDidMoveToHostWindow
 {
     [[[self frameView] documentView] viewDidMoveToHostWindow];
-    [[self childFrames] makeObjectsPerformSelector:@selector(_viewDidMoveToHostWindow)];
+    // It's OK to use the internal version because this method is
+    // guaranteed not to change the set of frames.
+    [[self _internalChildFrames] makeObjectsPerformSelector:@selector(_viewDidMoveToHostWindow)];
 }
 
 - (void)_reloadAllowingStaleDataWithOverrideEncoding:(NSString *)encoding
@@ -2187,7 +2217,9 @@ static CFAbsoluteTime _timeOfLastCompletedLoad;
     [_private->bridge saveDocumentState];
     [self _saveScrollPositionToItem:[_private currentItem]];
 
-    NSArray *frames = [self childFrames];
+    // It's OK to use the internal version because this method is
+    // guaranteed not to change the set of frames.
+    NSArray *frames = [self _internalChildFrames];
     int count = [frames count];
     int i;
     for (i = 0; i < count; i++) {
@@ -2426,7 +2458,9 @@ static CFAbsoluteTime _timeOfLastCompletedLoad;
         return [[self _bridge] numPendingOrLoadingRequests];
 
     num = [[self _bridge] numPendingOrLoadingRequests];
-    NSArray *children = [self childFrames];
+    // It's OK to use the internal version because this method is
+    // guaranteed not to change the set of frames.
+    NSArray *children = [self _internalChildFrames];
     int i, count = [children count];
     WebFrame *child;
     for (i = 0; i < count; i++){
@@ -2462,6 +2496,11 @@ static CFAbsoluteTime _timeOfLastCompletedLoad;
     }
 }
 
+- (NSArray *)_internalChildFrames
+{
+    return _private->children;
+}
+
 @end
 
 @implementation WebFrame (WebInternal)
index 1e0f0f5..43523af 100644 (file)
@@ -144,8 +144,7 @@ extern NSString *WebPageCacheDocumentViewKey;
 - (void)_transitionToLayoutAcceptable;
 - (WebFrameState)_state;
 - (void)_setState:(WebFrameState)newState;
-+ (void)_recursiveCheckCompleteFromFrame:(WebFrame *)fromFrame;
-- (void)_isLoadComplete;
+- (void)_checkLoadCompleteForThisFrame;
 - (void)_checkLoadComplete;
 - (void)_timedLayout:userInfo;
 - (WebBridge *)_bridge;
@@ -219,4 +218,6 @@ extern NSString *WebPageCacheDocumentViewKey;
 
 - (void)_reloadForPluginChanges;
 
+- (NSArray *)_internalChildFrames;
+
 @end
index d6e5919..eabe428 100644 (file)
@@ -669,7 +669,9 @@ NSString *_WebMainFrameURLKey =         @"mainFrameURL";
     if ([frame provisionalDataSource] == dataSource)
         return frame;
 
-    frames = [frame childFrames];
+    // It's safe to use the internal version because we know this
+    // function will not change the set of frames
+    frames = [frame _internalChildFrames];
     count = [frames count];
     for (i = 0; i < count; i++){
         aFrame = [frames objectAtIndex: i];
@@ -699,7 +701,9 @@ NSString *_WebMainFrameURLKey =         @"mainFrameURL";
     if ([frame frameView] == aView)
         return frame;
 
-    frames = [frame childFrames];
+    // It's safe to use the internal version because we know this
+    // function will not change the set of frames
+    frames = [frame _internalChildFrames];
     count = [frames count];
     for (i = 0; i < count; i++){
         aFrame = [frames objectAtIndex: i];