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

Cache Attributes, Tag Name and Value on Element creation #377

Merged

Conversation

masc-it
Copy link
Contributor

@masc-it masc-it commented Feb 18, 2023

Generally, we want to access an Element attributes and basic props when we create it, so why calling get_attributes() when we can already cache useful dom information, during the same Element instantation? I found out that it is literally a zero cost operation.

Also

  • get_attributes() calls get_description() which has a depth of 100
  • From my experiments, on long and complex pages it saves a lot of overhead (even couple of seconds per page)

Open point for another time

  • Why do el.get_description() and tab.describe_node() have a depth of 100? Maybe we can parametrize it

- calling get_description() over and over it is a waste (depth 100)
@Billy-Sheppard
Copy link
Collaborator

This seems sensible, thanks!

As for your last point I don't have any direct familiarity with those methods - but I don't see any reason why they can't take a parameter - perhaps its worth adding this functionality as a new method as not to break API - something like get_description_with_depth(&self, depth: Option<usize>) -> Result<DOM::Node> {}

@mdrokz
Copy link
Collaborator

mdrokz commented Feb 20, 2023

This seems sensible, thanks!

As for your last point I don't have any direct familiarity with those methods - but I don't see any reason why they can't take a parameter - perhaps its worth adding this functionality as a new method as not to break API - something like get_description_with_depth(&self, depth: Option<usize>) -> Result<DOM::Node> {}

This makes sense as well to ensure API compatibility otherwise everything looks good. @Billy-Sheppard you can merge this if you want

@mdrokz mdrokz added the enhancement New feature or request label Feb 20, 2023
@Billy-Sheppard Billy-Sheppard merged commit 3aa701f into rust-headless-chrome:main Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants