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

ElementsCollection performance using iterators #641

Closed
CaBocuk opened this Issue Dec 5, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@CaBocuk

CaBocuk commented Dec 5, 2017

The problem

Once I had to iterate through an Elements collection of 100+ elements I faced that it takes too much time (~10 seconds).

Details

So I investigated the issue and found that when we do the iteration - Selenide uses SelenideElementIterator to iterate BySelectorCollection. And it calls BySelectorCollection's method getActualElements() on EVERY hasNext() and next() call!

So i tried an experiment: I've changed the code of BySelectorCollection::getActualElements to:

  public List<WebElement> getActualElements() {
    SearchContext searchContext = parent == null ? getWebDriver() : parent;
    List<WebElement> elements = WebElementSelector.instance.findElements(searchContext, selector);
    System.out.println("Running "+ ++counter + " iteration of getActualElements(). Size = " +elements.size());
    return elements;
  }

and tested it on:

public static void main(String...args){
      Configuration.browser = "chrome";
      open("https://yandex.by/video/");
      $$("span").forEach(item -> item.is(visible));
  }

As an output I got: Running 365 iteration of getActualElements(). Size = 182
So for iterating through collection of 182 elements it finds 365 * 182 = 66430 elements

Holding experiments with elements amount I figured out the the amount found elements is (2*N^2 + N), where N - amount of elements in collection.
So for iterating through collection of 500 elements Selenide needs to find 500,500 elements :)

Should it really behave like that?

Tell us about your environment

  • Selenide Version: 4.8
  • Chrome: 62.0.3202.94
  • Browser Driver Version: automatically downloaded
@asolntsev

This comment has been minimized.

Show comment
Hide comment
@asolntsev

asolntsev Dec 21, 2017

Contributor

@CaBocuk Thank you for reporting the issue. We definitely need to fix it.

Contributor

asolntsev commented Dec 21, 2017

@CaBocuk Thank you for reporting the issue. We definitely need to fix it.

CaBocuk pushed a commit to CaBocuk/selenide that referenced this issue Dec 24, 2017

CaBocuk pushed a commit to CaBocuk/selenide that referenced this issue Dec 24, 2017

@CaBocuk

This comment has been minimized.

Show comment
Hide comment
@CaBocuk

CaBocuk Dec 24, 2017

Fixed it in #653
If the changes are ok, we can close it I think 😃

CaBocuk commented Dec 24, 2017

Fixed it in #653
If the changes are ok, we can close it I think 😃

@asolntsev asolntsev self-assigned this Dec 30, 2017

@asolntsev asolntsev added this to the 4.10 milestone Dec 30, 2017

@asolntsev asolntsev closed this in d1aee69 Jan 3, 2018

asolntsev added a commit that referenced this issue Jul 4, 2018

#696 reload collection elements on every call
partially revert changes done in #641, #672 and #277

asolntsev added a commit that referenced this issue Jul 4, 2018

#696 reload collection elements on every call
partially revert changes done in #641, #672 and #277

@asolntsev asolntsev referenced this issue Jul 4, 2018

Merged

#696 reload collection elements on every call #762

3 of 3 tasks complete

asolntsev added a commit that referenced this issue Jul 13, 2018

#696 reload collection elements on every call
partially revert changes done in #641, #672 and #277
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment