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

feat: Increase Xpath Lookup Performance #666

Merged
merged 3 commits into from
Feb 23, 2023
Merged

feat: Increase Xpath Lookup Performance #666

merged 3 commits into from
Feb 23, 2023

Conversation

Dan-Maor
Copy link
Collaborator

I’ve ran some general tests using a vanilla XCUITest project to investigate the impact of using block predicates, and it appears that the impact is severe, especially when allElementsBoundByIndex returns multiple results.

For example, consider searching for XCUIElementTypeButton elements using a string predicate:
[NSPredicate predicateWithFormat:@"elementType == 9”]
Versus a block predicate:
[NSPredicate predicateWithBlock:^BOOL(id<XCUIElementAttributes> _Nullable evaluatedObject, NSDictionary<NSString *,id> * _Nullable bindings) { return evaluatedObject.elementType == XCUIElementTypeButton; }];

When testing the above predicates 50 times using the Apple Clock app on an iPhone 8 real device running iOS 15.7.2 (6 applicable elements are located), using the string-based predicate the average execution time is 90ms, whereas executing a query based on a block predicate takes 140ms on average.

Similarly, using the same predicates on the Settings app of the same device 50 times (67 applicable elements are located), using the string predicate takes 265ms on average to execute the query, whereas the block predicate query takes 552ms on average to execute.

This PR adds the fb_uid method as a property of the XCElementSnapshot class, allowing it to be used with string predicates.

My tests showed a significant performance increase when using this method, for example - using the Xpath //XCUIElementTypeButton in the clock app takes 750ms to execute in the current state, whereas with the new implementation it is now executed nearly twice as fast at 400ms on average.

This also helps with the issue raised in appium/appium#18085 where deep nested elements cause snapshot retrieval to fail when increasing the maxSnapshotDepth to a higher value to locate such elements. I’ve built a simple app with a deep-nested structure (54 UIViews inside each other) and increased the snapshot depth to 100:

  • Without the modification, attempting to take a snapshot of the app to search for the 54th nested element failed (Enqueue Failure: Failed to resolve query: Error kAXErrorIllegalArgument getting snapshot for element) due to a CoreFoundation error in testmanagerd (too many nested dictionaries).
  • With the modification I was able to locate the element successfully.

I believe this modification is an important one since, aside from the beneficial performance increase, will help users with deep-nested iOS apps structures to use Appium.

@jlipps
Copy link
Member

jlipps commented Feb 22, 2023

wow, amazing find @Dan-Maor ! folks have been struggling with the nesting issue for a long time.

+ (void)load
{
Class XCElementSnapshotCls = objc_lookUpClass("XCElementSnapshot");
if (XCElementSnapshotCls != nil)

Choose a reason for hiding this comment

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

I would rather let it fail in such case because other features would anyway stop working if this property is not assigned successfully

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Replaced with NSAssert

Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

lgtm, great to know of the performance difference in the predicate method...

@mykola-mokhnach
Copy link

mykola-mokhnach commented Feb 22, 2023

IMHO it is never a good idea to use XPath location unless there is absolutely no other way to find an element (e.g. axes or special XPath functions are needed)

@KazuCocoa
Copy link
Member

Agree

@Dan-Maor
Copy link
Collaborator Author

Completely agree that Xpath is the worst option as well.

@Dan-Maor Dan-Maor merged commit 1696f4b into master Feb 23, 2023
@Dan-Maor Dan-Maor deleted the xpath_performance branch February 23, 2023 08:39
github-actions bot pushed a commit that referenced this pull request Feb 23, 2023
## [4.13.0](v4.12.2...v4.13.0) (2023-02-23)

### Features

* Increase Xpath Lookup Performance ([#666](#666)) ([1696f4b](1696f4b))
@github-actions
Copy link

🎉 This PR is included in version 4.13.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@hdesai-dave
Copy link

hdesai-dave commented Mar 9, 2023

Can we backport this change to appium 1.22.x since appium 2.0 is still in beta? @Dan-Maor @jlipps @mykola-mokhnach @KazuCocoa

@jlipps
Copy link
Member

jlipps commented Mar 10, 2023

@hdesai-dave Appium 2 is the only supported version of Appium. It's only in beta since the documentation is not yet fully fully complete (but it's 99% there). For all intents and purposes Appium 2 is out of beta and should be the only version used going forward. All that to say, we won't be backporting anything to 1.x.

@Jon889
Copy link

Jon889 commented Mar 23, 2023

Does this, block predicates having worse performance, mean that the usage of block predicates for iOS predicates sent from the appium client also have a negative performance impact? I'm new here, but I expected the predicates sent by the client to sent directly to the native XCUITest predicate functionality.

NSPredicate *formattedPredicate = [NSPredicate fb_snapshotBlockPredicateWithPredicate:predicate];

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

Successfully merging this pull request may close these issues.

6 participants