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

feat(query): support for batching of UI updates as the result of a database "array" loading #212

Open
Venryx opened this Issue Jul 28, 2017 · 2 comments

Comments

@Venryx

Venryx commented Jul 28, 2017

What is the current behavior?

I have a collection in my database, similar to this:

items: [
  "item1",
  "item2",
  "item3",
  ...
]

In my UI, I have a parent container element, and inside of it, boxes for each of the children. I use @Connect() on the parent and children elements.

However, the parent element requests/accesses/checks the data of all the children. This causes all of the children data to be requested from Firebase (however, as individual items rather than just the collection in one go, for technical reasons). It then gets sent to the app, and react-redux-firebase places it into the store. So far so good.

However, we get performance issues when we have many items, because react-redux-firebase downloads and places the items into the store one by one -- from what I can tell in the profiler. This means that the parent container element gets rendered many times during the loading/storing of the item data.

At first glance it seems like the solution is to have the parent element request all the child data in one go. However, I'm using a deep, function-tree-based data-retrieval system, so switching to "in one go" requesting would remove a lot of convenience benefits that currently exist.

Another thought is to use a library like https://github.com/tappleby/redux-batched-subscribe. However, I tried, and it doesn't reduce the update count significantly. I'm pretty sure it's because it only batches UI updates for a given database entry being added to the store -- and in my case, this base number of data entries/requests is itself too high.

What I'm suggesting is a feature by which react-redux-firebase tracks how often certain components (probably marked using an opt-in decorator) get re-rendered based on react-redux-firebase store updates, and if it sees that a component has been rendering very frequently for a sequence of add-entry-to-store actions, it could "suspend rerendering" for that component until the sequence of actions is complete.

It might be difficult to implement, so I understand it might not actually ever get implemented. But I figured I would bring it up in case it seems of use to other developers.

(it becomes a pretty big problem in larger apps, where there are hundreds of redux-connected components on-screen at once, and they all have the potential to react to common-data changes in the database part of the store -- combined with the common-data changing hundreds of times all at once because of a batch of entries being received at basically the same time but entered sequentially)

@prescottprue

This comment has been minimized.

Show comment
Hide comment
@prescottprue

prescottprue Jul 28, 2017

Owner

@Venryx Thanks for the suggestion! Performance of single views with tons of data updates has been something we have been noticing and starting to look into at work (we use this in production for our platform). After we do a more thorough investigation, I will better be able to determine a feature/bugfix and its associated timeline.

Really like the idea of another HOC to help track and maybe even control this, so that will be kept in mind.

Owner

prescottprue commented Jul 28, 2017

@Venryx Thanks for the suggestion! Performance of single views with tons of data updates has been something we have been noticing and starting to look into at work (we use this in production for our platform). After we do a more thorough investigation, I will better be able to determine a feature/bugfix and its associated timeline.

Really like the idea of another HOC to help track and maybe even control this, so that will be kept in mind.

@Venryx

This comment has been minimized.

Show comment
Hide comment
@Venryx

Venryx Jul 28, 2017

Got a quick solution working:

// have the code below run after the store is set up (but before components are created)
// ==========

let actionTypeBufferInfos = {
	"@@reactReduxFirebase/START": {time: 300},
	"@@reactReduxFirebase/SET": {time: 300},
};
let actionTypeLastDispatchTimes = {};
let actionTypeBufferedActions = {};

let oldDispatch = store.dispatch;
store.dispatch = action=> {
	let timeSinceLastDispatch = Date.now() - (actionTypeLastDispatchTimes[action.type] || 0);
	let bufferInfo = actionTypeBufferInfos[action.type];

	// if we're not supposed to buffer this action type, or it's been long enough since last dispatch of this type
	if (bufferInfo == null || timeSinceLastDispatch >= bufferInfo.time) {
		// dispatch action right away
		oldDispatch(action);
		actionTypeLastDispatchTimes[action.type] = Date.now();
	}
	// else, buffer action to be dispatched later
	else {
		// if timer not started, start it now
		if (actionTypeBufferedActions[action.type] == null) {
			setTimeout(()=> {
				// now that wait is over, apply any buffered event-triggers
				let combinedAction = {type: "ApplyActionSet", actions: actionTypeBufferedActions[action.type]};
				oldDispatch(combinedAction);

				actionTypeLastDispatchTimes[action.type] = Date.now();
				actionTypeBufferedActions[action.type] = null;
			}, (actionTypeLastDispatchTimes[action.type] + bufferInfo.time) - Date.now());
		}

		// add action to buffer, to be run when timer ends
		actionTypeBufferedActions[action.type] = (actionTypeBufferedActions[action.type] || []).concat(action);
	}
};

You also have to add a special handler to your root reducer, so it will take the "ApplyActionSet" action, take out the actions in the inner array, and apply each one sequentially.

In my project, this looks like:

const store = createStore(MakeRootReducer(), [...]);

function MakeRootReducer() {
	const innerReducer = combineReducers({
		main: MainReducer,
		firebase: firebaseStateReducer,
		[...]
	});

	return (state, rootAction)=> {
		let actions = rootAction.type == "ApplyActionSet" ? rootAction.actions : [rootAction];

		let result = state;
		for (let action of actions) {
			result = innerReducer(result, action);
		}
		return result;
	};
}

Its performance impact on my project is quite large. Loading a tree of ~200 nodes went down from 60 seconds to 11!

Inspecting the actions dispatched in dev-tools shows that it's now batching some sets of over 200 START/SET actions into just one -- keeping the components that depend on multiple entries in that data from having to continually rerender.

Venryx commented Jul 28, 2017

Got a quick solution working:

// have the code below run after the store is set up (but before components are created)
// ==========

let actionTypeBufferInfos = {
	"@@reactReduxFirebase/START": {time: 300},
	"@@reactReduxFirebase/SET": {time: 300},
};
let actionTypeLastDispatchTimes = {};
let actionTypeBufferedActions = {};

let oldDispatch = store.dispatch;
store.dispatch = action=> {
	let timeSinceLastDispatch = Date.now() - (actionTypeLastDispatchTimes[action.type] || 0);
	let bufferInfo = actionTypeBufferInfos[action.type];

	// if we're not supposed to buffer this action type, or it's been long enough since last dispatch of this type
	if (bufferInfo == null || timeSinceLastDispatch >= bufferInfo.time) {
		// dispatch action right away
		oldDispatch(action);
		actionTypeLastDispatchTimes[action.type] = Date.now();
	}
	// else, buffer action to be dispatched later
	else {
		// if timer not started, start it now
		if (actionTypeBufferedActions[action.type] == null) {
			setTimeout(()=> {
				// now that wait is over, apply any buffered event-triggers
				let combinedAction = {type: "ApplyActionSet", actions: actionTypeBufferedActions[action.type]};
				oldDispatch(combinedAction);

				actionTypeLastDispatchTimes[action.type] = Date.now();
				actionTypeBufferedActions[action.type] = null;
			}, (actionTypeLastDispatchTimes[action.type] + bufferInfo.time) - Date.now());
		}

		// add action to buffer, to be run when timer ends
		actionTypeBufferedActions[action.type] = (actionTypeBufferedActions[action.type] || []).concat(action);
	}
};

You also have to add a special handler to your root reducer, so it will take the "ApplyActionSet" action, take out the actions in the inner array, and apply each one sequentially.

In my project, this looks like:

const store = createStore(MakeRootReducer(), [...]);

function MakeRootReducer() {
	const innerReducer = combineReducers({
		main: MainReducer,
		firebase: firebaseStateReducer,
		[...]
	});

	return (state, rootAction)=> {
		let actions = rootAction.type == "ApplyActionSet" ? rootAction.actions : [rootAction];

		let result = state;
		for (let action of actions) {
			result = innerReducer(result, action);
		}
		return result;
	};
}

Its performance impact on my project is quite large. Loading a tree of ~200 nodes went down from 60 seconds to 11!

Inspecting the actions dispatched in dev-tools shows that it's now batching some sets of over 200 START/SET actions into just one -- keeping the components that depend on multiple entries in that data from having to continually rerender.

@prescottprue prescottprue added this to the v1.6.0 milestone Aug 9, 2017

prescottprue added a commit that referenced this issue Aug 30, 2017

Roadmap and README updated.
* #212 added to roadmap
* #126 added to roadmap
* Readme updated with messaging

@prescottprue prescottprue changed the title from Improved support for batching of UI updates as the result of a database "array" loading to feat(query): support for batching of UI updates as the result of a database "array" loading Sep 2, 2017

@prescottprue prescottprue modified the milestones: v1.6.0, v2.0.0 Oct 27, 2017

@prescottprue prescottprue added this to To Do in v2.0.0 Oct 29, 2017

@prescottprue prescottprue removed this from To Do in v2.0.0 Dec 22, 2017

@prescottprue prescottprue added this to To Do in Future Versions Dec 22, 2017

@prescottprue prescottprue modified the milestones: v2.0.0, Future Versions Dec 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment