Merge pull request #590 from matrix-org/luke/fix-membereventlistsummary-performance

Improve the performance of MemberEventListSummary

src/components/structures/MessagePanel.js

@@ -298,6 +298,15 @@ module.exports = React.createClass({
             // Wrap consecutive member events in a ListSummary
             if (isMembershipChange(mxEv)) {
                 let ts1 = mxEv.getTs();
+                // Ensure that the key of the MemberEventListSummary does not change with new
+                // member events. This will prevent it from being re-created unnecessarily, and
+                // instead will allow new props to be provided. In turn, the shouldComponentUpdate
+                // method on MELS can be used to prevent unnecessary renderings.
+                //
+                // Whilst back-paginating with a MELS at the top of the panel, prevEvent will be null,
+                // so use the key "membereventlistsummary-initial". Otherwise, use the ID of the first
+                // membership event, which will not change during forward pagination.
+                const key = "membereventlistsummary-" + (prevEvent ? mxEv.getId() : "initial");
 
                 if (this._wantsDateSeparator(prevEvent, ts1)) {
                     let dateSeparator = <li key={ts1+'~'}><DateSeparator key={ts1+'~'} ts={ts1}/></li>;
@@ -334,7 +343,7 @@ module.exports = React.createClass({
 
                 ret.push(
                     <MemberEventListSummary
-                        key={mxEv.getId()}
+                        key={key}
                         events={summarisedEvents}
                         data-scroll-token={eventId}>
                             {eventTiles}

src/components/views/elements/MemberEventListSummary.js

@@ -42,7 +42,7 @@ module.exports = React.createClass({
         return {
             summaryLength: 3,
             threshold: 3,
-            avatarsMaxLength: 5
+            avatarsMaxLength: 5,
         };
     },
 
@@ -144,88 +144,107 @@ module.exports = React.createClass({
         );
     },
 
-    render: function() {
+    shouldComponentUpdate: function(nextProps, nextState) {
-        let summary = null;
+        // Update if
-
+        //  - The number of summarised events has changed
-        // Reorder events so that joins come before leaves
+        //  - or if the summary is currently expanded
-        let eventsToRender = this.props.events;
+        //  - or if the summary is about to toggle to become collapsed
-
+        //  - or if there are fewEvents, meaning the child eventTiles are shown as-is
-        // Create an array of events that are not "cancelled-out" by another
+        return (
-        // A join of sender S is cancelled out by a leave of sender S etc.
+            nextProps.events.length !== this.props.events.length ||
-        let filteredEvents = [];
+            this.state.expanded || nextState.expanded ||
-        let senders = new Set(eventsToRender.map((e) => e.getSender()));
+            nextProps.events.length < this.props.threshold
-        senders.forEach(
-            (userId) => {
-                let userEvents = eventsToRender.filter((e) => {
-                    return e.getSender() === userId;
-                });
-
-                // NB: These may be the same event, in which case the lastEvent is used
-                // because prev_content should != content
-                let firstEvent = userEvents[0];
-                let lastEvent = userEvents[userEvents.length - 1];
-
-                // Membership BEFORE eventsToRender
-                let previousMembership = firstEvent.getPrevContent().membership || "leave";
-
-                // Otherwise, if the last membership event differs from previousMembership,
-                // use that.
-                if (previousMembership !== lastEvent.getContent().membership) {
-                    filteredEvents.push(lastEvent);
-                }
-            }
         );
+    },
 
-        let joinAndLeft = (eventsToRender.length - filteredEvents.length) / 2;
+    render: function() {
-        if (joinAndLeft <= 0 || joinAndLeft % 1 !== 0) {
+        let eventsToRender = this.props.events;
-            joinAndLeft = null;
-        }
-
-        let joinEvents = filteredEvents.filter((ev) => {
-            return ev.event.content.membership === 'join';
-        });
-
-        let leaveEvents = filteredEvents.filter((ev) => {
-            return ev.event.content.membership === 'leave';
-        });
-
         let fewEvents = eventsToRender.length < this.props.threshold;
         let expanded = this.state.expanded || fewEvents;
-        let expandedEvents = null;
 
+        let expandedEvents = null;
         if (expanded) {
             expandedEvents = this.props.children;
         }
 
-        let avatars = this._renderAvatars(joinEvents.concat(leaveEvents));
+        if (fewEvents) {
-
+            return (
-        let toggleButton = null;
+                <div className="mx_MemberEventListSummary">
-        let summaryContainer = null;
+                    {expandedEvents}
-        if (!fewEvents) {
-            summary = this._renderSummary(joinEvents, leaveEvents);
-            toggleButton = (
-                <a className="mx_MemberEventListSummary_toggle" onClick={this._toggleSummary}>
-                    {expanded ? 'collapse' : 'expand'}
-                </a>
-            );
-            let plural = (joinEvents.length + leaveEvents.length > 0) ? 'others' : 'users';
-            let noun = (joinAndLeft === 1 ? 'user' : plural);
-
-            summaryContainer = (
-                <div className="mx_EventTile_line">
-                    <div className="mx_EventTile_info">
-                        <span className="mx_MemberEventListSummary_avatars">
-                            {avatars}
-                        </span>
-                        <span className="mx_TextualEvent mx_MemberEventListSummary_summary">
-                            {summary}{joinAndLeft ? joinAndLeft + ' ' + noun + ' joined and left' : ''}
-                        </span>&nbsp;
-                        {toggleButton}
-                    </div>
                 </div>
             );
         }
 
+        // Map user IDs to the first and last member events in eventsToRender for each user
+        let userEvents = {
+            // $userId : {first : e0, last : e1}
+        };
+
+        eventsToRender.forEach((e) => {
+            const userId = e.getSender();
+            // Initialise a user's events
+            if (!userEvents[userId]) {
+                userEvents[userId] = {first: null, last: null};
+            }
+            if (!userEvents[userId].first) {
+                userEvents[userId].first = e;
+            }
+            userEvents[userId].last = e;
+        });
+
+        // Populate the join/leave event arrays with events that represent what happened
+        // overall to a user's membership. If no events are added to either array for a
+        // particular user, they will be considered a user that "joined and left".
+        let joinEvents = [];
+        let leaveEvents = [];
+        let joinedAndLeft = 0;
+        let senders = Object.keys(userEvents);
+        senders.forEach(
+            (userId) => {
+                let firstEvent = userEvents[userId].first;
+                let lastEvent = userEvents[userId].last;
+
+                // Membership BEFORE eventsToRender
+                let previousMembership = firstEvent.getPrevContent().membership || "leave";
+
+                // If the last membership event differs from previousMembership, use that.
+                if (previousMembership !== lastEvent.getContent().membership) {
+                    if (lastEvent.event.content.membership === 'join') {
+                        joinEvents.push(lastEvent);
+                    } else if (lastEvent.event.content.membership === 'leave') {
+                        leaveEvents.push(lastEvent);
+                    }
+                } else {
+                    // Increment the number of users whose membership change was nil overall
+                    joinedAndLeft++;
+                }
+            }
+        );
+
+        let avatars = this._renderAvatars(joinEvents.concat(leaveEvents));
+        let summary = this._renderSummary(joinEvents, leaveEvents);
+        let toggleButton = (
+            <a className="mx_MemberEventListSummary_toggle" onClick={this._toggleSummary}>
+                {expanded ? 'collapse' : 'expand'}
+            </a>
+        );
+        let plural = (joinEvents.length + leaveEvents.length > 0) ? 'others' : 'users';
+        let noun = (joinedAndLeft === 1 ? 'user' : plural);
+
+        let summaryContainer = (
+            <div className="mx_EventTile_line">
+                <div className="mx_EventTile_info">
+                    <span className="mx_MemberEventListSummary_avatars">
+                        {avatars}
+                    </span>
+                    <span className="mx_TextualEvent mx_MemberEventListSummary_summary">
+                        {summary}{joinedAndLeft ? joinedAndLeft + ' ' + noun + ' joined and left' : ''}
+                    </span>&nbsp;
+                    {toggleButton}
+                </div>
+            </div>
+        );
+
         return (
             <div className="mx_MemberEventListSummary">
                 {summaryContainer}