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

Selectors performance gap #163

Closed
sergey-tihon opened this issue Aug 20, 2014 · 20 comments
Closed

Selectors performance gap #163

sergey-tihon opened this issue Aug 20, 2014 · 20 comments

Comments

@sergey-tihon
Copy link
Collaborator

Today we had Dojo-Canopy-2048 in Minsk (Thanks to @mathias-brandewinder for the fun) and have faced with interesting behavior of our bot.

When we add calls to lost() and won() functions in Interactions.fs performance of emulation is dramatically going down.

We profiled our app and have found that the reason is in canopy selectors. On each game loop iteration we are trying to check existence of UI elements that are not exist until the end of the game.
image

Since canopy does not know format of selector, you need to try all one by one until the end of defaultFinders seq. But elements do not exist and all finders return nothing. Nevertheless, you need to run all finders on each call.

On the other hand, when you use Selenium you can specify kind of selector that you use. So the following code snippet works dramatically faster (but it is not idiomatic canopy):

    let gameEnded =
        let driver = lazy(Seq.head browsers)
        let css = ".game-message.game-won, .game-message.game-over"
        let selector = OpenQA.Selenium.By.CssSelector(css)
        fun () ->
            driver.Value.FindElements(selector).Count > 0

I think that canopy should provide and API to specify kind of selector and allow users to have performance comparable with Selenium code.

@lefthandedgoat
Copy link
Owner

Sergey!

You could specify the list of finders you want to use here: https://github.com/lefthandedgoat/canopy/blob/master/src/canopy/configuration.fs#L21

If the app only uses css selectors just make the list have only css.

Another way, which they way I think you are suggestion is to allow a 'hint' to tell canopy to not try all selectors because the provided selector is a css/xpath/etc selector

Instead of:
"#country" << "Germany"
which would potentially cycle through all selectors
you could, with some enhancements:
css "#country" << "Germany"
which would tell canopy that its a css selector and optimize to not try everything, only try with css selector.

Number 2 sound reasonable?

Option three I guess would be to allow the By.CssSelector syntax like so:
By.CssSelector("#country") << "Germany"

@sergey-tihon
Copy link
Collaborator Author

Personally i like syntax

css "#country" << "Germany"

Is it possible to analyze selector string and identify type of selector?

@lefthandedgoat
Copy link
Owner

I could do that, but its possible to get it wrong i would think. I could try the guess 2-3 times and then fall back to the full list of selectors. Do you think that is a reasonable compromise?
I like that syntax best too.

@sergey-tihon
Copy link
Collaborator Author

Do you know frequency of use non-CSS selectors? If others selectors are not very popular...

We can prepare two OOTB modes:

  • CSS selectors only (default)
  • All availabe selectors

And mention in documentations that Canopy use css selectors only (by default). if you want use all selectors it can hurt performance but you can easily do this using

configuration.useAllFilders <- true

Also you could setup custom list of finders using configuredFinders.

Does it sound reasonable?

@lefthandedgoat
Copy link
Owner

I know that I use 5 of the 6 heavily. I will think on some of this tomorrow and some bench marking of various solutions.

@lefthandedgoat
Copy link
Owner

I made some optimizations on branch performance-163: 6202a0d

Test code:

url testpage

many 1750 (fun _ -> 
    let elem = someElement ".bad-selector"    
    elem.IsNone === true
)

Results for 1750 runs:
Default: 77.1 seconds
Turn off find in IFrame: 60.8 seconds
Finder optimization only: 25.2 seconds
Both optimizations: 14.4 seconds

Right now both optimizations are on by default and will fall back to old behavior for reliable finds if it fails with optimizations 3 times

What are your thoughts?

@lefthandedgoat
Copy link
Owner

I've been thinking more about this. I am concerned that the solution I have has the potential to cause more problems than it solves, especially until the definitions of checking to see if something is a certain type of selector are better.

Maybe just adding ability to specify the type of selector is best. The case canopy-2048 has where you are expecting the element to not be there in 99.9% of cases is kind of an edge case. Definitely want to improve it though.

someElement <| css ".bad-selector"

I do think the ability to turn iFrame attempts off is good, so people can turn it off if they know that their app has no iFrames

Still thinking on this one =)

@lefthandedgoat
Copy link
Owner

Can you give a link to your repo to reproduce this?

I just updated to Mathias master Canopy2048 branch and profiled it.

This are my results:

Overall runtime 107 seconds

findByJquery/findByCSS etc totaled 1.1 seconds, so ~1% of overall time.

column was 12.3%
row was 9.02%

sergey-tihon pushed a commit to sergey-tihon/Dojo-Canopy-2048 that referenced this issue Aug 25, 2014
@sergey-tihon
Copy link
Collaborator Author

Sounds interesting...
Turning off search in iFrames sound reasonable for me in most cases. I think it would be nice to prepare new page on Canopy site with guidance how to improve performance with detailed description what and how can be turned off/optimized.

My bot is here https://github.com/sergey-tihon/Dojo-Canopy-2048/tree/performance
You can compare performance by altering gameEnded condition.

@lefthandedgoat
Copy link
Owner

Yah a page about performance/configuration would be good.

I will look at your project tonight. Thanks!

@lefthandedgoat
Copy link
Owner

Ok cool, definite difference between the two on yours. Mathias Canopy2048 (not dojo) is using a slower more complex algorithm so checking win/loss is not as obvious. With the algorithm you are using its much more obvious. Good find. I will continue to think on it and try to implement explicit hints tomorrow.

I think I can also make it faster by turning off the logging that is used for coverage report too.

@lefthandedgoat
Copy link
Owner

Sergey,

I finally came up with a solution, which while not properly functional (due to mutable global, I've already messed up with a global browser etc), was easy to implement and works well:

47641ff

Basically I added 2 new flags:

  1. to turn off selector/url logging for coverage report
  2. to turn off checking in Iframe if you know your code does not have any

I also added some helpers to that will register a string as using a specific finder. You can see those in canopy.fs lines 780-786 and usage at 213

I manually built this dll and dropped it in to my nuget package canopy folder for Dojo-Canopy-20048 and made these changes:

lefthandedgoat/Dojo-Canopy-2048@ee3d6ba

The css hint gives the performance like the change you made, and the other flags being turned on helped a little too.

Give me your thoughts. If this is a reasonable enough solution I will do a new nuget build and get it out into the wild!

@sergey-tihon
Copy link
Collaborator Author

I like the idea, but in some cases it can lead to unexpected behavior in some cases (see my comments on the code)
I still think that we need to create a new page in canopy site with manual how improve performance.

@lefthandedgoat
Copy link
Owner

Your improvement ideas are good, and yes an additional page about performance is a good.

@lefthandedgoat
Copy link
Owner

Sergey,

I made the changes you suggested, all good ideas: bf6efe4
and
945a8f3

These for the dojo:
lefthandedgoat/Dojo-Canopy-2048@0deab59

let me know what you think.

@sergey-tihon
Copy link
Collaborator Author

Looks good to me
image
Thank you!

@lefthandedgoat
Copy link
Owner

Cool, my goal is to have a new build out tonight. Would you like to send a PR to Mathias for the Dojo or want me to?

@sergey-tihon
Copy link
Collaborator Author

I think that this improvement is your merit, so PR is yours ;)

@lefthandedgoat
Copy link
Owner

Done and published. I am also sending a PR to the dojo for Mathias!

Thanks Sergey!!!

@sergey-tihon
Copy link
Collaborator Author

Thank you!

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

No branches or pull requests

2 participants