Skip to content
This repository has been archived by the owner on Dec 26, 2019. It is now read-only.

Use NSProxy as base class for FBApplicationProcessProxy #979

Closed
wants to merge 3 commits into from

Conversation

fr0l
Copy link
Contributor

@fr0l fr0l commented Aug 15, 2018

Allows to forward NSKeyValueObserverRegistration methods.

Fixes issue #975 — Allows to work with Xcode 10 beta 5 and beta 6.

Tested for Xcode 9.4.1 as well

@@ -122,7 +122,7 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
return;
}
XCUIApplicationProcess *applicationProcess = change[NSKeyValueChangeNewKey];
if (!applicationProcess || ![applicationProcess isMemberOfClass:XCUIApplicationProcess.class]) {
if (!applicationProcess || [applicationProcess isProxy] || ![applicationProcess isMemberOfClass:XCUIApplicationProcess.class]) {

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

proxy.applicationProcess = applicationProcess;
return proxy;
}

- (instancetype)init {
return self;

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

@@ -45,9 +52,16 @@ - (void)waitForQuiescenceIncludingAnimationsIdle:(BOOL)includeAnimations
[self.applicationProcess waitForQuiescenceIncludingAnimationsIdle:includeAnimations];
}

- (id)forwardingTargetForSelector:(SEL)aSelector
Copy link
Contributor

Choose a reason for hiding this comment

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

Why kill forwardingTargetForSelector? it is faster then invocation right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't done any benchmarking, and implemented the only two required methods. if this is faster, then let's use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

From https://developer.apple.com/documentation/objectivec/nsobject/1418855-forwardingtargetforselector

This method gives an object a chance to redirect an unknown message sent to it before the much more expensive forwardInvocation: machinery takes over.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Simple benchmark with 10 million invocations answers the question "How much more expensive?" :-) 0.3 seconds vs 26 seconds


@end

@implementation FBApplicationProcessProxyTest
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@fr0l
Copy link
Contributor Author

fr0l commented Aug 20, 2018

@marekcirkos Thanks for the hint on the forwardingTargetForSelector method.

I did benchmarking — basically a test which was calling XCTAssertEqual([proxy proxiedMethod], 1); 10 million times.

And received surprising and impressive results.

FBApplicationProcessProxy WITH implemented forwardingTargetForSelector FBApplicationProcessProxy WITHOUT implemented forwardingTargetForSelector
~ 0.3 seconds ~ 26 seconds

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

marekcirkos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@marekcirkos
Copy link
Contributor

Nice work! Thanks for contributing!

@jamesvanhorn
Copy link

Thanks for quick fix

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

Successfully merging this pull request may close these issues.

None yet

4 participants