Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed improvements #323

Merged
merged 9 commits into from May 21, 2011
Merged

Speed improvements #323

merged 9 commits into from May 21, 2011

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented May 17, 2011

This pull has a few different things in it.

  • The time of the 'superfluous visual effects' (set in appearance prefs) has been reduced for the Bezel Plugin.
    This gives the appearance of QS being snappier (that's half of it eh?) and the closing time was just too long and has been annoying me for a while
  • The time for 3rd pane resizing has been reduced - again for the Bezel interface. the setFrame:display:animate method calls animationResizeTime dynamically to get the resize time. By default it is set to 0.2s but Apple states it could change at any time. I just did an over ride of this method and set it to 0.01s - makes the 3rd pane resizing snappier ;)
  • Altered the implementation of the fast enumeration stuff.
    I'm not entirely sure if this will speed up Quicksilver, but I think so, for these reasons:
    • Apple states in the fast enumeration discussion that

      The enumeration is considerably more efficient than, for example, using NSEnumerator directly

    • Previously, NSEnumerators were being created then fast-enumerated on. I don't think that this is as efficient as it is now: The NSEnumerators are now being created directly, and I guess the OS has a better idea of when to release them (straight away) than if they were created by the code (as it was).

    • Also, this is exactly how Apple writes fast enumeration: As per their docs, it looks nicer and is a lot easier to read (IMO)

Finally, since there are lots and lots of fast enumerations changed, it's be good if you guys could glance over them - I've debugged the ones that I thought may be 'dodgy' and it all seems good.

Here are a few places where you may care to take a look:

  • L625 of QSObject_FileHandling.m - replaced foreachkey(k,x,y) - a macro defined in QSMacros.h
    Pretty sure I've done this right and I've stepped through it a few times in the debugger. Looks good.
  • L478 QSObject.m - I couldn't break on this, so couldn't check it. I think it's absolutely fine though (don't see why now)

Not fast enumeration related

  • L183 NSFileManager_BLTRExtensions - stringByResolvingSymlinksInPath is called when stringByStandardizingPath finds a symlink (check docs)
  • L52 QSResizingInterfaceController.m - !inderctOptional is first instead of in the else{} since this is the most likely case (indirect is not optional). Yeah. Won't make much (any) a difference but it looks/flows nicer IMO :)

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented May 17, 2011

hmm...care to elaborate? :-)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented May 17, 2011

@HenningJ patience! :P
Accidentally hit enter when making the pull so it was made without any comments. See above now

@skurfer
Copy link
Member

@skurfer skurfer commented May 18, 2011

This causes a merge conflict with your web search icons branch in Quicksilver/Code-QuickStepCore/QSObject_URLHandling.m.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented May 18, 2011

Right you are. the favicon method doesn't exist in my web search icons branch. I can revert that change here and resubmit - shouldn't cause any problems.

@skurfer
Copy link
Member

@skurfer skurfer commented May 18, 2011

I think I’ve found a problem with this. I’ll dig into it more as I can. Not 100% of the time, but pretty consistently reproducible: If I enter text in the first pane then tab to the second, the interface locks until you restart. The console says

2011-05-18 14:12:56.526 Quicksilver[10698:a0b] -[NSCFString countByEnumeratingWithState:objects:count:]: unrecognized selector sent to instance 0x16311950
2011-05-18 14:12:56.529 Quicksilver[10698:a0b] HIToolbox: ignoring exception '-[NSCFString countByEnumeratingWithState:objects:count:]: unrecognized selector sent to instance 0x16311950' that raised inside Carbon event dispatch
(
    0   CoreFoundation                      0x951916ba __raiseError + 410
    1   libobjc.A.dylib                     0x9550a509 objc_exception_throw + 56
    2   CoreFoundation                      0x951de90b -[NSObject(NSObject) doesNotRecognizeSelector:] + 187
    3   CoreFoundation                      0x95137c36 ___forwarding___ + 950
    4   CoreFoundation                      0x95137802 _CF_forwarding_prep_0 + 50
    5   CoreFoundation                      0x951309a3 -[NSMutableSet addObjectsFromArray:] + 83
    6   QSCore                              0x0010b429 -[QSLibrarian arrayForType:] + 425
    7   Web Search Module                   0x164c5fd6 0x0 + 374104022
    8   QSInterface                         0x001b0dcb -[QSInterfaceController searchObjectChanged:] + 491
    9   Nostromo                            0x167cc20e -[QSNostromoInterfaceController searchObjectChanged:] + 78
    10  Foundation                          0x95f983bf _nsnote_callback + 176
    11  CoreFoundation                      0x95118793 __CFXNotificationPost + 947
    12  CoreFoundation                      0x9511819a _CFXNotificationPostNotification + 186
    13  Foundation                          0x95f8d25c -[NSNotificationCenter postNotificationName:object:userInfo:] + 128
    14  Foundation                          0x95f9a669 -[NSNotificationCenter postNotificationName:object:] + 56
    15  QSInterface                         0x001c9964 -[QSSearchObjectView selectObjectValue:] + 244
    16  QSInterface                         0x001c9d91 -[QSSearchObjectView selectIndex:] + 273
    17  QSInterface                         0x001cbab4 -[QSSearchObjectView performSearchFor:from:] + 756
    18  QSInterface                         0x001cb789 -[QSSearchObjectView performSearch:] + 201
    19  Foundation                          0x95fc68d4 __NSFireTimer + 141
    20  Foundation                          0x961005c9 -[NSCFTimer fire] + 102
    21  QSInterface                         0x001cc598 -[QSSearchObjectView partialStringChanged] + 808
    22  QSInterface                         0x001cf80c -[QSSearchObjectView insertText:] + 188
    23  AppKit                              0x903f144a -[NSTextInputContext insertText:replacementRange:] + 421
    24  AppKit                              0x903ef3e7 -[NSTextInputContext handleTSMEvent:] + 2657
    25  AppKit                              0x903ee975 _NSTSMEventHandler + 209
    26  HIToolbox                           0x9240ac2f _ZL23DispatchEventToHandlersP14EventTargetRecP14OpaqueEventRefP14HandlerCallRec + 1567
    27  HIToolbox                           0x92409ef6 _ZL30SendEventToEventTargetInternalP14OpaqueEventRefP20OpaqueEventTargetRefP14HandlerCallRec + 411
    28  HIToolbox                           0x9242c7f3 SendEventToEventTarget + 52
    29  HIToolbox                           0x9248e9de SendTSMEvent + 82
    30  HIToolbox                           0x9248e35b SendUnicodeTextAEToUnicodeDoc + 700
    31  HIToolbox                           0x9248df65 TSMKeyEvent + 998
    32  HIToolbox                           0x9247ee32 TSMProcessRawKeyEvent + 2515
    33  AppKit                              0x903ee254 -[NSTextInputContext handleEvent:] + 1453
    34  AppKit                              0x903ea0a1 -[NSView interpretKeyEvents:] + 209
    35  QSInterface                         0x001cd349 -[QSSearchObjectView keyDown:] + 1849
    36  AppKit                              0x9031ed38 -[NSWindow sendEvent:] + 5757
    37  QSEffects                           0x000dfd00 -[QSWindow sendEvent:] + 784
    38  AppKit                              0x90237817 -[NSApplication sendEvent:] + 6431
    39  Quicksilver                         0x0000392e -[QSApp sendEvent:] + 1790
    40  AppKit                              0x901cb2a7 -[NSApplication run] + 917
    41  AppKit                              0x901c32d9 NSApplicationMain + 574
    42  Quicksilver                         0x00020256 main + 294
    43  Quicksilver                         0x00002755 start + 53
)

@skurfer
Copy link
Member

@skurfer skurfer commented May 18, 2011

Noticed this seems to be in QSLibrarian, which the Yojimbo module uses. Can’t right arrow into Yojimbo.app either.

5/18/11 2:19:50 PM  Quicksilver[10749]  -[NSCFString countByEnumeratingWithState:objects:count:]: unrecognized selector sent to instance 0x2bb2a0

Still investigating.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented May 19, 2011

What interface?
Most likely cause is trying to enumerate an array (or dictionary) etc. that has a count of 0 in this case

@skurfer
Copy link
Member

@skurfer skurfer commented May 19, 2011

My first thought was that it’s my new flaky interface, but I was able to reproduce it in several others.

Just did this to confirm:

  1. Changed interface to Bezel (built-in)
  2. Quit Quicksilver
  3. Switched to your speed branch
  4. Cleaned all targets
  5. Build & Run
  6. Enter text in the first pane
  7. Tab to the second pane

Quicksilver will be useless after that.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented May 20, 2011

Right you are. I'd accidentally enumerated the keys instead of the objects. I'll check over the rest of the code now to make sure I haven't done this more than once

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented May 20, 2011

...and not enumerating the dictionary's values instead of the keys would be why the last NSEnumerator wasn't working and somebody had added a 'caution, don't change this one' note.

@skurfer
Copy link
Member

@skurfer skurfer commented May 20, 2011

OK, I’ve built this in and will run with it for a while.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented May 20, 2011

Sweet. Sounds good :)

Once this is done, I'll add another pull with changes to the DEBUG stuff.

On 21 May 2011 00:00, skurfer <
reply@reply.github.com>wrote:

OK, Ive built this in and will run with it for a while.

Reply to this email directly or view it on GitHub:
#323 (comment)

@@ -612,11 +611,11 @@ static float searchSpeed = 0.0;
}

- (NSMutableArray *)scoredArrayForString:(NSString *)string {
return [self scoredArrayForString:string inSet:nil];
return [self scoredArrayForString:string inSet:nil mnemonicsOnly:NO];
}

- (NSMutableArray *)scoredArrayForString:(NSString *)string inNamedSet:(NSString *)setName {
Copy link
Contributor

@fheckl fheckl May 20, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really related to this pull request but maybe we should mark this method as @deprecated (to speak a little Java here) since setName is not used...

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented May 20, 2011

Yeah, I've done that now :)

@skurfer
Copy link
Member

@skurfer skurfer commented May 21, 2011

I haven’t encountered any more stability problems with this. I also looked through every one of the diffs and it looks good.

Looking forward to the upcoming DEBUG changes.

skurfer added a commit that referenced this issue May 21, 2011
@skurfer skurfer merged commit 92efd35 into quicksilver:master May 21, 2011
@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented May 21, 2011

Seems to have reduced QS RAM usage by ~400kB when comparing to 59 (may have
been other changes we've done along the way I guess). Not bad though -
that's a whole 0.01% of my RAM I'm saving :)

On 21 May 2011 14:13, skurfer <
reply@reply.github.com>wrote:

I havent encountered any more stability problems with this. I also looked
through every one of the diffs and it looks good.

Looking forward to the upcoming DEBUG changes.

Reply to this email directly or view it on GitHub:
#323 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants