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

Performance optimisations #208

Open
martinpengellyphillips opened this issue Apr 16, 2023 · 14 comments
Open

Performance optimisations #208

martinpengellyphillips opened this issue Apr 16, 2023 · 14 comments
Labels

Comments

@martinpengellyphillips
Copy link
Contributor

I've been porting one of my personal projects to SUID and have noticed a significant performance degradation (especially on mobile) as a result. For example, rendering a list with listitem buttons has noticeable (>250ms) lag compared to the same native div setup.

Are there known performance improvements that can be applied / has any investigation into improving performance been done already?

@juanrgm
Copy link
Member

juanrgm commented Apr 16, 2023

How many elements are you rendering? Could you share the code?

@martinpengellyphillips
Copy link
Contributor Author

Take a look at https://getfinnick.com/debug which compares suid list to native (similar elements). There are 300 entries there to make it easy to notice, but the lag can be felt at 100+ entries on mobile. Seems to be a lot of jank in the suid version.

Screenshot 2023-04-17 at 22 53 10

@juanrgm
Copy link
Member

juanrgm commented Apr 18, 2023

I will check it out.

The source code would speed up testing.

@martinpengellyphillips
Copy link
Contributor Author

Ah, yeah, forgot to add that! Here you go:

import {
  Box,
  Button,
  List,
  ListItem,
  ListItemButton,
  ListItemText,
  Stack,
} from "@suid/material";
import { For, VoidComponent, createSignal } from "solid-js";
import { getRandomId } from "../random";

const createRandomList = () =>
  [...Array(300)].map((_) => ({ name: `Entry ${getRandomId()}` }));

const DebugPage = () => {
  const [entriesA, setEntriesA] = createSignal(createRandomList());
  const [entriesB, setEntriesB] = createSignal(createRandomList());
  return (
    <Stack direction="row" gap={3}>
      <Box>
        <Button
          fullWidth
          variant="contained"
          onClick={() => setEntriesA(createRandomList())}
        >
          Re-init SUID list
        </Button>
        <Box height="80vh" overflow="auto">
          <SuidListExample entries={entriesA()} />
        </Box>
      </Box>
      <Box>
        <Button
          fullWidth
          variant="contained"
          onClick={() => setEntriesB(createRandomList())}
        >
          Re-init native list
        </Button>
        <Box height="80vh" overflow="auto">
          <RawListExample entries={entriesB()} />
        </Box>
      </Box>
    </Stack>
  );
};

const SuidListExample: VoidComponent<{ entries: any[] }> = (props) => {
  return (
    <List>
      <For each={props.entries}>
        {(entry) => (
          <ListItem>
            <ListItemButton>
              <ListItemText>{entry.name}</ListItemText>
            </ListItemButton>
          </ListItem>
        )}
      </For>
    </List>
  );
};

const RawListExample: VoidComponent<{ entries: any[] }> = (props) => {
  return (
    <ul style={{ padding: "8px 0px" }}>
      <For each={props.entries}>
        {(entry) => (
          <li style={{ padding: "8px 16px" }}>
            <button style={{ padding: "8px 16px" }}>
              <p style={{ margin: "4px 0" }}>
                <span>{entry.name}</span>
              </p>
            </button>
          </li>
        )}
      </For>
    </ul>
  );
};

export default DebugPage;

@juanrgm
Copy link
Member

juanrgm commented Apr 20, 2023

I wrote the test with React-MUI for comparing: https://stackblitz.com/edit/react-qq4ans?file=demo.tsx

@martinpengellyphillips
Copy link
Contributor Author

Looks like mui and native are comparable there so specific to suid?

I did notice lots of context lookups in the suid version, but I haven't seen context be an issue before in general.

@juanrgm
Copy link
Member

juanrgm commented Apr 22, 2023

I prepared other test for debugging: https://stackblitz.com/edit/angular-1kaidf?file=src%2FApp.tsx

We can't compare native elements with components in SolidJS because they are totally different, a SolidJS component will never have the same performance.

I have detected some bottlenecks:

@martinpengellyphillips
Copy link
Contributor Author

Anything I can do to help here?

@juanrgm
Copy link
Member

juanrgm commented Apr 27, 2023

solidjs/solid#1703 (comment)

@martinpengellyphillips
Copy link
Contributor Author

martinpengellyphillips commented May 10, 2023

I've applied the current latest versions of SUID and SolidJS now. I'm not seeing a noticeable improvement - is there more still to do here?

Update

Nevermind! I see there is ongoing commits on performance over the last few days. Let me know if I can help test at all :)

@juanrgm
Copy link
Member

juanrgm commented May 11, 2023

The other PR is here: solidjs/solid#1710.

Still, SolidJS has other "limitations" that can directly affect how SUID is built (modular components): solidjs/solid#1399.

@juanrgm juanrgm added the WIP Work in process label May 11, 2023
@martinpengellyphillips
Copy link
Contributor Author

@juanrgm fyi with the latest releases it seems that things like selected on <ListItemButton> no longer work - the styles are not applied. In the screen below, when the 'x' is moving it also means I have passed selected={true} to the <ListItemButton> but the styling no longer highlights. It did work in the version just prior (e.g. @suid/material@0.12.1).

suid-selection-issue

@evertheylen
Copy link

I'm also having performance problems so I thought it could be useful to add my own example here. It is the minimal version of code currently in production, and the performance is a real problem right now.

Stackblitz link

It renders three versions of the same 300 "shopping list" entries. I recorded some timings (without debug mode, on Firefox):

  • SUID: ~857ms
  • Solid with basic divs etc: ~5ms
  • Direct DOM manipulation: ~4ms

While SUID visually looks a lot better, being 2 orders of magnitude slower is too high of a performance cost.

Note: 300 entries is more than users actually have, it is just a benchmark. Users on less powerful devices with a more complex shopping list have problems starting from 50 entries. (I thought 5ms was already slow but it seems you can barely beat it with direct DOM manipulation.)

@evertheylen
Copy link

Another update. I wondered whether this was really unique to SUID or if other component libraries also have this problem. So I included Park UI and Kobalte (which seem to be the only other mature options besides SUID).

Stackblitz link

Kobalte runs in ~168ms, and Park runs in ~382ms. Even that seems too slow to me. Is there something inherent to component libraries that makes them slow in Solid.js? (Next up, I should probably benchmark the same setup in other frontend frameworks.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants