1416096 - Find out a way to reduce the frequency of React UI update

Status: RESOLVED WONTFIX

(DevTools :: Netmonitor, enhancement, P1)

Description

[Ricky Chien [:rickychien] (inactive)]

The current update flow of rendering a network requests:


NetworkEvent -> action (updateRequest action) -> reducer -> Redux state -> Redux notifies connected components -> React UI update.

When network requests pour into Netmonitor, React UI update will happen very frequently. As a result, every single update will cascade props to descendant components. No matter the UI get updated or not, it's likely to trigger a lots of component checks (shouldComponentUpdate)

Thus, the number of actions depends on how many network events come from back-end. 


Option 1:

If we can somehow avoid invoking updateRequest() action in each NetworkEvent, then we can skip each action -> reducer procedure. (We also know that there are some perf issues in Immutable.js)

Moving state.requests to a separate object (a plain JS object), which can avoid recoding all network requests into Redux state so that lead to too many unnecessary UI update. And then we sync the network record object to Redux state periodically e.g 100ms to tell Redux to notify a React UI update.

The tradeoff of this approach is that we're going to rewrite a part of reducer, Redux state, firefox-connector & firefox-data-provider. It could be a nice opportunity for us to refactor the data structure of state.requests because some of data fetching might be inefficient and redundant.


Option 2:

Alternatively, trying to debounce Redux state listener with enhancer like redux-batched-subscribe [1] might be possible to mitigate our performance bottleneck. I'm going to investigate this part first since it could be simpler approach for us to just introduce a new enhancer but solve the root cause.

[1] https://github.com/tappleby/redux-batched-subscribe

Comment 1

[Ricky Chien [:rickychien] (inactive)]

Alex, do you think Option 1 is reasonable? Will it block your this work (bug 1404913)?

Comment 2

[Alexandre Poirot [:ochameau]]

(In reply to Ricky Chien [:rickychien] from comment #0)
> The current update flow of rendering a network requests:
> 
> 
> 
> Option 1:
> 
> If we can somehow avoid invoking updateRequest() action in each
> NetworkEvent, then we can skip each action -> reducer procedure. (We also
> know that there are some perf issues in Immutable.js)

Actually, we should not fire that many updateRequest calls.
In the Data Provider, we do aggregate them:
https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/connector/firefox-data-provider.js#495-505

Everytime we fetch a field like requestHeaders, responseCookies, ...
We check for isRequestPayloadReady(), and only when all the fields are fetched,
we do call actions.updateRequest() with all these fields set.

So. For each request we should call addRequest and updateRequest once.

> Option 2:
> 
> Alternatively, trying to debounce Redux state listener with enhancer like
> redux-batched-subscribe [1] might be possible to mitigate our performance
> bottleneck. I'm going to investigate this part first since it could be
> simpler approach for us to just introduce a new enhancer but solve the root
> cause.

As you saw in the other bug, we have some throttle/batching already in place.


> NetworkEvent -> action (updateRequest action) -> reducer -> Redux state ->
> Redux notifies connected components -> React UI update.

I think you should look closer into this pipeline and what really happens in detail in each individual step.
Mike's work on React shouldComponentUpdate may help working on the very last step (react ui update).

Comment 3

[Ricky Chien [:rickychien] (inactive)]

Based on comment 2, close this issue.