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/differences compared with redux #50

Closed
benwill opened this issue May 25, 2020 · 18 comments
Closed

Performance/differences compared with redux #50

benwill opened this issue May 25, 2020 · 18 comments

Comments

@benwill
Copy link

benwill commented May 25, 2020

I can provide an example if need be, but I've been looking at using your library rather than redux for a component I'm building. One major difference I notice is that, whenever any state changes, every single component in the tree does run the functional components (can see this by adding console logs within them, for all of your examples)

My current implementation using redux, and useSelector, only actually logs the components that have changed based on state. e.g. the textbox being updated, whereas switching to this, I actually see every single print a console log out, even if it is not impacted.

So is this a key difference? Would you expect the logic within components to still be ran? even if the rendering is optimised (Which i'd expect react to take care of anyway?)

@dai-shi
Copy link
Owner

dai-shi commented May 25, 2020

I see what you mean, very good finding.
This is a long story, but what I tell you is this is not something I'm very happy with.
If you are interested, please try v1.2.0. It should work as you expect.

Now, I needed to change the logic in v1.3.0, to avoid a React warning that is introduced in 16.13.
v1.3.0 is to follow the warning and essentially for Concurrent Mode. You could use v1.2.0 as long as in Legacy Mode.
The performance in v1.2.0 was comparable or slightly better than react-redux, but v1.3.0 would perform worse, as you guess.

My plan is to move to v2 #43 once React releases a new version with useMutableSource.
Meanwhile, we need to live with v1. Have you experienced any real issue with performance drop? I can try investigating it if there's a real example that performs bad in v1.3.0 but not in v1.2.0. I know any benchmarks would show difference though.

Lastly, while a render function runs, it will not be committed unless state is changed. It means if a render function is computationally heavy, the performance may drop a lot.

Sorry for the inconvenience. Unless you need to stick with a specific React version, it should be ok to keep using this lib and wait for the future.

@benwill
Copy link
Author

benwill commented May 25, 2020

Very quick reply thank you, for my particular usecase, i'm trying to make sure I nail the performance early on (looking at building a way of building complex nested layouts with drag and drop, so keen to make it as optimised as possible).

Its more of a project at the moment, so isn't a big rush, is this something you feel will be behaving in the way described after v2 is released? Anything stopping me from using v2 at the moment?

In terms of a performance drop, I believe majority of the cost i'd have is rendering charts for example, which wouldn't be an issue here, however if I had to manipulate data heavily to render the charts (lots of loops or alot of data), I could see this being problematic.

Thanks again for your reply, I'm particularly keen on this since I don't want to be tied to redux, worth noting if theres anything I can help with, let me know :)

@dai-shi
Copy link
Owner

dai-shi commented May 25, 2020

v2 is already alpha released: https://www.npmjs.com/package/react-tracked/v/2.0.0-alpha.4
You need experimental builds of react and react-dom.
(Note I have some difficulties with the latest build. #43 (comment) Maybe not for your app.)

You can try v2-alpha to see how it is going to be like. I actually encourage you for this trial, because that could reveal some bugs I don't notice now. This is not for production.

Feel free to ask here, if you have some issues with trying v2.

@benwill
Copy link
Author

benwill commented May 25, 2020

I've just had a try on one of the examples (examples:01_minimal) on the v2 branch, and still see the same thing (changing the textbox for example also fires console.logs inside the counter)

@dai-shi
Copy link
Owner

dai-shi commented May 25, 2020

OK, I just tried and confirmed the latest v2 branch, which doesn't work.
The same reason #43 (comment) for all examples.

Anyway, I understand the behavior. As you observed, it triggers an extra render function. If you change count and text one by one repeatedly, both render functions run. So, it doesn't behave like v1.2.0, but it's not like v1.3.0 either. I'm not sure if there's a bug on my end, but as useMutableSource is changing, let's wait for the new experimental release.

Meanwhile, let me run js-framework-benchmark.

@dai-shi
Copy link
Owner

dai-shi commented May 25, 2020

Here's the result. Hm, there doesn't seem much difference between v1.2.0 and v1.4.0.
According to the result, it would be quite safe to say the extra render function call is trivial.
If one has heavy computation in render, they could use useMemo anyway (no matter if it uses react-tracked or not).

image

@ShamansCoding
Copy link

Is it correct, that the current stable version of the library doesn't prevent all unnecessary re-renders of the components?

I use React Context within my app with useReducer. It was quite clean way to replace native React Context provider with this library. But in the profiler, i don't see any changes in rendering of the components.

Everything seems to work stable, but all component tree seems to be re-rendered as it does with React Context usage.

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2020

@ShamansCoding I understand the latest stable version (v1.4.1) doesn't seem to behave intuitively. It may call the render function but will not commit. So, you might want to compare the behavior with useEffect.

useEffect(() => {
  console.log('rendered');
});

If you use v1.2.0 and it works as expected, my assumption should be correct.

But in the profiler, i don't see any changes in rendering of the components.

I'm not sure what this means.

There might still be room for improvements in v1.4.1. If you could share a codesandbox, I can investigate it.

Thanks!

@ShamansCoding
Copy link

ShamansCoding commented Jul 1, 2020

Thanks for the quick response!

I meant React dev tools extension. And its option "Highlight updates when components render."

I've tried to use v1.2 and v1.4.1 and the result is the same. Then I trigger any changes in the application general context store(which is replaced by the library), all components shown as re-rendered. Even those components that don't use part of the store that has changed.

I also placed some console logs inside re-rendered components and they show the same picture.

Below is what I've done.

Code before:

import React, { createContext, useReducer } from 'react';
import { userInitialState, userActions } from 'actions/userActions';
import { drawerInitialState, drawerActions } from 'actions/drawerActions';

const initialState = {
  ...userInitialState,
  ...drawerInitialState,
};

const Actions = {
  ...userActions,
  ...drawerActions,
};

function reducer(store, action) {
  const selectedAction = Actions[action.type];
  const { payload } = action;
  const update = selectedAction({ store, payload });
  return { ...store, ...update };
}

const initialContext = { store: initialState, dispatch: null };

const StoreContext = createContext({ ...initialContext });

function StoreProvider({ children }) {
  const [initialStore, dispatch] = useReducer(reducer, initialState);

  return (
    <StoreContext.Provider value={{ initialStore, dispatch }}>
      {children}
    </StoreContext.Provider>
  );
}

export { StoreContext, StoreProvider };

Code after:

import React, { createContext, useReducer } from 'react';
import { createContainer } from 'react-tracked';
import { userInitialState, userActions } from 'actions/userActions';
import { drawerInitialState, drawerActions } from 'actions/drawerActions';

const initialState = {
  ...userInitialState,
  ...drawerInitialState,
};

const Actions = {
  ...userActions,
  ...drawerActions,
};

function reducer(store, action) {
  const selectedAction = Actions[action.type];
  const { payload } = action;
  const update = selectedAction({ store, payload });
  return { ...store, ...update };
}

const useValue = () => useReducer(reducer, initialState);

export const {
  Provider: StoreProvider,
  useTrackedState: useStore,
  useUpdate: useDispatch,
} = createContainer(useValue);

@dai-shi
Copy link
Owner

dai-shi commented Jul 1, 2020

I meant React dev tools extension. And its option "Highlight updates when components render."

Oh, I see. Yeah, I noticed it before too. TBH, I don't know how it works.

I've tried to use v1.2 and v1.4.1 and the result is the same.

Yeah. I'm pretty sure it's not our issue. It's either the dev tools doesn't take the hack (changedBits = 0) into account, or it highlights all context consumers regardless of it.

I also placed some console logs inside re-rendered components and they show the same picture.

Are you sure? Try it with v1.2.0. With v1.4.1, please use useEffect to console log.

@thatsnotnice
Copy link

thatsnotnice commented Jul 3, 2020

@dai-shi Besides the fact that the semantics of console.log() belongs anyway to useEffect, I'd like to understand one aspect.

  1. CompA selects slice X of state.
  2. CompB selects slice Y of state.
  3. CompC dispatches an update to slice Y.
  4. CompA gets re-executed and returns according to slice X.
  5. CompB gets re-executed and returns according to slice Y.

From here, I assume React's commit/reconciliation process somehow discards output from CompA after diffing before/after outputs.
What I'm wondering is how pricey can this process be, especially in the case of a single application-wide state object.

Moreover, I'm trying to keep components as small as I can, especially state-consumers ones. So far the performance impact is not that much of an issue.
But what I noticed instead is that class-based components (yeah... got some legacy stuff to deal with), if instantiated from state consumers, get a full UI re-render that React's commit/reconciliation process is not-capable to optimize/prevent.
A situation that disappears with v1.2.0.

I guess it's just an implication of this

@dai-shi
Copy link
Owner

dai-shi commented Jul 3, 2020

Well, thanks for asking!

(I'd keep to describe v1.2.0 behavior.)

  1. CompA gets re-executed and returns according to slice X.

This is not correct. Because only Y is changed, CompA will not be executed. Hense, no reconciliation work is done by React.

Moreover, I'm trying to keep components as small as I can, especially state-consumers ones. So far the performance impact is not that much of an issue.

If you split contexts too, it's good. You don't need any global state solution.

But what I noticed instead is that class-based components

I'm not so sure how class components and function components differ in this case.

(I'm not so sure if I can answer well your to your question, so follow-ups are welcome.)

@ShamansCoding
Copy link

ShamansCoding commented Jul 3, 2020

Are you sure? Try it with v1.2.0. With v1.4.1, please use useEffect to console log.

I looked more carefully for my case, and I found out what is happening. The re-renders were caused because the direct parent of the component is using a part of the store which is changed. So all child components get affected by this re-render. Then I change the part of the state, which is not used by the parent component, there is no additional re-renders. Both in v1.2.0 and v1.4.1. When a parent is using the part of the state, re-render is triggered for all children.

@dai-shi
Copy link
Owner

dai-shi commented Jul 3, 2020

Thanks for your investigation.

When a parent is using the part of the state, re-render is triggered for all children.

Yeah, that's React default behavior. It may confuse developers sometimes.

@dai-shi
Copy link
Owner

dai-shi commented Jul 7, 2020

Maybe related discussion: facebook/react#19200

@axeljeremy7
Copy link

@dai-shi which version is the best to choose i am using v1.4.2 , do you think is recommended to stay up to there or move to the latest one or v1.4.1 to avoid re - render?

@dai-shi
Copy link
Owner

dai-shi commented Dec 25, 2020

@axeljeremy7

Short answer

Please use the latest version. If you find any issues, please report it.

Long answer

  • v1.5 is a big rewrite, and it basically deprecates v1.3.* and v1.4.*.
  • v1.2 was a nice implementation, but now v1.5 is implemented without changedBits hack, v1.5 would be better in a long term for production.
  • Performance-wise, all versions should be fairly good compared to react-redux. You wouldn't notice any difference in realistic apps.
  • I haven't run benchmarks with v1.5 yet. If someone finds any issues in v1.5, I can take a look.
  • I plan to release v1.6 soon, but it's just adding a new exported function. Nothing would be changed for the existing code from v1.5.

@dai-shi
Copy link
Owner

dai-shi commented Dec 26, 2020

Let me close this. Please file a new issue for a new discussion.

@dai-shi dai-shi closed this as completed Dec 26, 2020
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

5 participants