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

perf(element): add one sweep element creation #109

Merged
merged 6 commits into from Feb 26, 2023
Merged

perf(element): add one sweep element creation #109

merged 6 commits into from Feb 26, 2023

Conversation

j-mendez
Copy link
Contributor

  1. major performance improvements on element creation

src/node.rs Outdated Show resolved Hide resolved
src/node.rs Outdated Show resolved Hide resolved
src/node.rs Outdated Show resolved Hide resolved
@j-mendez
Copy link
Contributor Author

@cfvescovo re-verted a.value.deref() back to prior. Can take a look later on updating to a match later or etc.

@cfvescovo
Copy link
Collaborator

Maybe we could match a.name.local.deref() and check if it is equal to id or class. The call to a.value().deref() should be inside each arm of our match. Performing it inside the match is better since it will only be called if needed (at most once per iteration).

@j-mendez
Copy link
Contributor Author

Maybe we could match a.name.local.deref() and check if it is equal to id or class. The call to a.value().deref() should be inside each arm of our match. Performing it inside the match is better since it will only be called if needed (at most once per iteration).

I like that, feel free to hop on the PR and make those changes or any, need to brb.

src/node.rs Outdated
}
classes.extend(value.split_whitespace().map(LocalName::from));
id = Some(LocalName::from(a.value.deref()));
} else if name_local == "class" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

was rushing and made this change without thinking. @adamreichold @cfvescovo feel free to squash the commits for this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will do, thanks

@cfvescovo
Copy link
Collaborator

Maybe we could match a.name.local.deref() and check if it is equal to id or class. The call to a.value().deref() should be inside each arm of our match. Performing it inside the match is better since it will only be called if needed (at most once per iteration).

I like that, feel free to hop on the PR and make those changes or any, need to brb.

I do not think I have permissions to edit this PR

@j-mendez
Copy link
Contributor Author

match a.name.local.deref()

ok, ill make the changes really quick should be simple

@j-mendez
Copy link
Contributor Author

Maybe we could match a.name.local.deref() and check if it is equal to id or class. The call to a.value().deref() should be inside each arm of our match. Performing it inside the match is better since it will only be called if needed (at most once per iteration).

I like that, feel free to hop on the PR and make those changes or any, need to brb.

I do not think I have permissions to edit this PR

@cfvescovo updated

@cfvescovo
Copy link
Collaborator

@j-mendez thank you

@cfvescovo cfvescovo merged commit aa479ea into causal-agent:master Feb 26, 2023
@j-mendez j-mendez deleted the perf/node-creation branch February 26, 2023 21:46
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.

None yet

3 participants