[system] Various perf gain experiments

[mnajdova]

Trying various micro-optimizations, nothing big found at this moment yet

[mui-pr-bot]

Details of bundle changes

Generated by 🚫 dangerJS against d68c7c0

[oliviertassinari]

@mnajdova What do you think about ignoring these metrics (removing them from the benchmark):

colors @material-ui/system  x 418,856 ops/sec ±2.23% (189 runs sampled)
colors styled-system x 957,951 ops/sec ±1.48% (190 runs sampled)
spaces @material-ui/system x 289,762 ops/sec ±1.25% (188 runs sampled)
spaces styled-system x 901,040 ops/sec ±1.76% (185 runs sampled)
compose @material-ui/system x 58,856 ops/sec ±0.20% (189 runs sampled)
compose styled-system x 224,670 ops/sec ±1.74% (184 runs sampled)

and focusing on the end-to-end version: @material-ui/core all-inclusive?

[oliviertassinari]

Trying various micro-optimizations, nothing big found at this moment yet.

@mnajdova We might want to rewrite the system to iterate on the props instead of calling each transformation. I think that Theme-UI and Chakra-UI have demonstrated that it can be faster unless it's their cache that helps the most. I think that it will make it easier to scale to custom transformations without impacting runtime cost. For instance, what if I want to add a gapX prop that adds when used.

& > * + * {
  margin-right: 8px;
}

@mbrookes a reference to #18459 (review).

[mnajdova]

@mnajdova We might want to rewrite the system to iterate on the props instead of calling each transformation. I think that Theme-UI and Chakra-UI have demonstrated that it can be faster unless it's their cache that helps the most. I think that it will make it easier to scale to custom transformations without impacting runtime cost. For instance, what if I want to add a gapX prop that adds when used.

This is how the sx style function works right now. Let's remove from it the support for all other box props and see if there will be some perf gain. Focusing only on that one can be more productive :)

[mnajdova]

We need deepmerge only if both source and target are objects. Otherwise we can override with source[key]

[oliviertassinari]

These look like overall improvements :).

One thing that could be interesting is to benchmark, on the Node.js side (what we call server) is the performance of the system raw, meaning sx transformation method vs. https://styled-system.com/css vs. import { css } from '@chakra-ui/styled-system'. The function that converts the superset of CSS into CSS. And remove the benchmark on the lower-level methods, like color, compose, etc.

[mnajdova]

One thing that could be interesting is to benchmark, on the Node.js side (what we call server) is the performance of the system raw, meaning sx transformation method vs. https://styled-system.com/css vs. import { css } from '@chakra-ui/styled-system'. The function that converts the superset of CSS into CSS. And remove the benchmark on the lower-level methods, like color, compose, etc.

I was trying this method already, seems like in order to be equivalent we need to run css and then the returned function. Anyway let me confirm one more time and replace the current benchmarks we have in place 👍

[mnajdova]

Opened #23702 for updating benchmarks. Will extract PRs from this one along te way for the changes we agree are good to go :) Will keep this one as a hub for trying all changes

[mnajdova]

After merging #23716 I think this can be opened as a standalone PR.

[oliviertassinari]

After merging #23716 I think this can be opened as a standalone PR.

@mnajdova Sounds good. I'm having a look at if I can improve the quality of the output of the benchmark test (I won't look into React.Profiler path as I know it's a direction you are already on).

[mnajdova]

No need for cloning

[mnajdova]

merge is more suited and performant for these use-cases than deepmerge - it returns early for null args + doesn't clone the first arg.

[mnajdova]

This was wrong previously, if the value is a function we need to resolve it and do the rest of the logic.

[mnajdova]

Updated tests for the function changes implemented here https://github.com/mui-org/material-ui/pull/23688/files#r530971112

[mnajdova]

@mnajdova Sounds good. I'm having a look at if I can improve the quality of the output of the benchmark test (I won't look into React.Profiler path as I know it's a direction you are already on).

@oliviertassinari trying the React.Profiler - #23731 results don't look much more stable either... :\

[oliviertassinari]

@oliviertassinari trying the React.Profiler - #23731 results don't look much more stable either... :\

I have noticed that when I switch windows during the run of the tests, I get a spike of std. Two ideas to reduce it, no idea if it works:

• /2 the number of components mounted per measurement, x2 the number of measurements.
• run the benchmark in a remote machine. We could maybe setup a machine in the cloud to run it from time to time.

But maybe it's already good enough. Maybe we should focus on identifying the slowest path, fix it, iterate. We can learn from existing alternative implementations.

[mnajdova]

But maybe it's already good enough. Maybe we should focus on identifying the slowest path, fix it, iterate. We can learn from existing alternative implementations.

Agree, this first round is just about small improvements that can immediately be fixed and seems like we are now comparable with theme-ui at least.

Next, I will try to see if restructuring the whole code will make much difference, but would like to have this one in first, to compare the difference based on these new numbers