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

Some front-end performance suggestions #23

Open
gbj opened this issue Apr 16, 2023 · 0 comments
Open

Some front-end performance suggestions #23

gbj opened this issue Apr 16, 2023 · 0 comments

Comments

@gbj
Copy link

gbj commented Apr 16, 2023

I saw your Reddit post and thought I'd take a quick look to see if there were any quick improvements that could be made for front-end performance.

All of these are just offered in a friendly way because I think your project is cool! Obviously no obligation to do anything about them.

Here are a few concrete ideas:

  • you can enable string interning on element tag names, attribute keys, and even listener names here, which will make element creation measurably faster (see wasm-bindgen docs here)
    let el = document.create_element(self.name).unwrap();
    for attr in &self.attrs {
    el.set_attribute(&attr.key, &attr.value).unwrap();
  • You can make element creation even faster by using Element.cloneNode() instead of Document.createElement() and caching elements by tag name — you can see details in my PR to Yew (Incremental performance improvements to element creation yewstack/yew#3169)
  • it looks like your diff function diffs the new virtual DOM against the existing real DOM, reading from the DOM to get the node name, content, attributes, etc. Reading from the DOM like this is quite expensive even in JS, and even more so in WASM because of the cost of copying those strings over. A more typical VDOM approach would be to keep the old VDOM around, diff the new VDOM against the old VDOM, and then apply patches to the real DOM only when something is changed.
    fn diff(&mut self, node: &mut Node) {
    let mut name = node.node_name();
    if name == "#text" && node.node_value().unwrap() == "" {
    *node = node.next_sibling().unwrap();
    name = node.node_name();
    }
    if self.name == name {
    let el = node.dyn_ref::<Element>().unwrap();
    let attributes = el.attributes();

I'm not sure what's going on with your "remove row" and haven't actually run the benchmark, but based on the size of the number I wonder whether it is removing a row and then shifting all the following rows. For example, if you remove a row at position 10 of 1000 you should be able just to remove row 10 — I wonder whether you are instead diffing each row against the next one, re-creating it, and then deleting the last row? (if you see what I mean)

With the exception of "remove row," you are already performing as well as React on this benchmark. So, great work so far, don't sweat it too much, and thanks for everything you're doing!

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

No branches or pull requests

1 participant