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

build(deps): fix unchecked lock and update deps #107

Merged
merged 3 commits into from
Feb 25, 2023
Merged

build(deps): fix unchecked lock and update deps #107

merged 3 commits into from
Feb 25, 2023

Conversation

j-mendez
Copy link
Member

  1. build(deps): update cssparser@0.29.6 and selector@0.24.0 (mem reduction, perf increase, and css3 syntax fixes )
  2. fix missing Cargo.lock file
  3. add lazy evaluation classes

Cargo.toml Outdated Show resolved Hide resolved
@j-mendez
Copy link
Member Author

@cfvescovo @cfvescovo .or is evaluated regardless if it is true. For this situation we do not need to create a new hashset eagerly and can move it lazily.

We can use or for this block in the element_ref and it makes sense since we want this to be fast during this execution.

    fn is_root(&self) -> bool {
        self.parent()
            .map_or_else(|| false, |parent| parent.value().is_document())
    }

@cfvescovo
Copy link
Member

I know but Rust's collections generally don't allocate on initialisation, so an eager String::new() or HashMap::new() doesn't really matter

@cfvescovo
Copy link
Member

However I see your point so I removed my comment

@j-mendez
Copy link
Member Author

j-mendez commented Feb 25, 2023

I know but Rust's collections generally don't allocate on initialisation, so an eager String::new() or HashMap::new() doesn't really matter

We want to leave the stack alone for this and should be fine to move it lazy. I can revert the change to keep it just with the build deps.

@cfvescovo
Copy link
Member

I was referencing this: https://stackoverflow.com/a/73785382

@cfvescovo
Copy link
Member

I know but Rust's collections generally don't allocate on initialisation, so an eager String::new() or HashMap::new() doesn't really matter

We want to leave the stack alone for this and should be fine to move it lazy. I can revert the change to keep it just with the build deps.

Keep your changes, I see your point. It won't make much of a difference but ok.

@j-mendez
Copy link
Member Author

@cfvescovo thank you for the fast replies - fixing the format issues now.

@cfvescovo
Copy link
Member

Cargo fmt is complaining, rerun the formatter

@cfvescovo
Copy link
Member

@j-mendez thank you!

@cfvescovo cfvescovo merged commit 6abb8cd into rust-scraper:master Feb 25, 2023
@EdJoPaTo EdJoPaTo mentioned this pull request Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants