Skip to content

Commit

Permalink
[Perf] Improved performance for useActivityTreeWithRenderer (#5163)
Browse files Browse the repository at this point in the history
* Early investigations

* Add skipCheckAccessibility locally

* Fix destructuring

* Remove unnecessary accessibility check

* Add performance test

* Rename

* Use Map.get instead of [].find

* Clean up tests

* Add entry

* Update PR number

* Typo

* Update CHANGELOG.md
  • Loading branch information
compulim committed May 8, 2024
1 parent d3a6f52 commit e44dd95
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 16 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Expand Up @@ -22,6 +22,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]

### Fixed

- Improved performance for `useActivityWithRenderer`, in PR [#5172](https://github.com/microsoft/BotFramework-WebChat/pull/5172), by [@OEvgeny](https://github.com/OEvgeny)
- Fixes [#5162](https://github.com/microsoft/BotFramework-WebChat/issues/5162). Improved performance for `useActivityTreeWithRenderer`, in PR [#5163](https://github.com/microsoft/BotFramework-WebChat/pull/5163), by [@compulim](https://github.com/compulim)

### Changed

- Bumped all dependencies to the latest versions, by [@compulim](https://github.com/compulim) in PR [#5174](https://github.com/microsoft/BotFramework-WebChat/pull/5174)
Expand Down Expand Up @@ -128,7 +133,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Fixes missing exports of `useNotifications`, in PR [#5148](https://github.com/microsoft/BotFramework-WebChat/pull/5148), by [@compulim](https://github.com/compulim)
- Fixes suggested actions keyboard navigation skips actions after suggested actions got updated, in PR [#5150](https://github.com/microsoft/BotFramework-WebChat/pull/5150), by [@OEvgeny](https://github.com/OEvgeny)
- Fixes [#5155](https://github.com/microsoft/BotFramework-WebChat/issues/5155). Fixed "Super constructor null of anonymous class is not a constructor" error in CDN bundle by bumping to [`webpack@5.91.0`](https://www.npmjs.com/package/webpack/v/5.91.0), in PR [#5156](https://github.com/microsoft/BotFramework-WebChat/pull/5156), by [@compulim](https://github.com/compulim)
- Improved performance for `useActivityWithRenderer`, in PR [#5172](https://github.com/microsoft/BotFramework-WebChat/pull/5172), by [@OEvgeny](https://github.com/OEvgeny)

### Changed

Expand Down
1 change: 1 addition & 0 deletions __tests__/html/performance/README.md
@@ -0,0 +1 @@
Tests in this folder may not have its corresponding `.js` file as we are still exploring how to write tests for tracking performance issues.
88 changes: 88 additions & 0 deletions __tests__/html/performance/manyMessages.batched.html
@@ -0,0 +1,88 @@
<!doctype html>
<html lang="en-US">
<head>
<link href="/assets/index.css" rel="stylesheet" type="text/css" />
<script crossorigin="anonymous" src="/test-harness.js"></script>
<script crossorigin="anonymous" src="/test-page-object.js"></script>
<script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
</head>
<body>
<main id="webchat"></main>
<script>
const NUM_MESSAGE = 200;
const QUORUM_SIZE = 20;

run(
async function () {
const {
WebChat: { ReactWebChat }
} = window; // Imports in UMD fashion.

const { directLine, store } = testHelpers.createDirectLineEmulator();

WebChat.renderWebChat({ directLine, store }, document.querySelector('main'));

await pageConditions.uiConnected();

const startAt = Date.now();
const timeForEachQuorum = [];
const promises = [];
let lastFlushAt = Date.now();

for (let index = 0; index < NUM_MESSAGE; index++) {
promises.push(
// Plain text message isolate dependencies on Markdown.
directLine.emulateIncomingActivity(
{ text: `Message ${index}.`, textFormat: 'plain', type: 'message' },
{ skipWait: true }
)
);

if (promises.length >= QUORUM_SIZE) {
await Promise.all(promises.splice(0));
await pageConditions.numActivitiesShown(index + 1);

const now = Date.now();

timeForEachQuorum.push(now - lastFlushAt);
lastFlushAt = now;
}
}

if (promises.length) {
await Promise.all(promises);

const now = Date.now();

timeForEachQuorum.push(now - lastFlushAt);
}

await pageConditions.numActivitiesShown(NUM_MESSAGE);

// Remove outliers.
timeForEachQuorum.shift();
timeForEachQuorum.shift();

const firstHalf = timeForEachQuorum.splice(0, timeForEachQuorum.length >> 1);

const sumOfFirstHalf = firstHalf.reduce((sum, time) => sum + time, 0);
const sumOfSecondHalf = timeForEachQuorum.reduce((sum, time) => sum + time, 0);

const averageOfFirstHalf = sumOfFirstHalf / firstHalf.length;
const averageOfSecondHalf = sumOfSecondHalf / timeForEachQuorum.length;

console.log(`Took ${Date.now() - startAt} ms.`, {
firstHalf,
secondHalf: timeForEachQuorum,
averageOfFirstHalf,
averageOfSecondHalf
});

// Time taken to render the first half of the chat history, should be 80% similar to the time taken to render the second half of the chat history.
expect(averageOfFirstHalf / averageOfSecondHalf).toBeGreaterThan(0.8);
},
{ skipCheckAccessibility: true }
);
</script>
</body>
</html>
46 changes: 46 additions & 0 deletions __tests__/html/performance/manyMessages.oneByOne.html
@@ -0,0 +1,46 @@
<!doctype html>
<html lang="en-US">
<head>
<link href="/assets/index.css" rel="stylesheet" type="text/css" />
<script crossorigin="anonymous" src="/test-harness.js"></script>
<script crossorigin="anonymous" src="/test-page-object.js"></script>
<script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
</head>
<body>
<main id="webchat"></main>
<script>
const NUM_MESSAGE = 400;
const QUORUM = 5;

run(async function () {
const {
WebChat: { ReactWebChat }
} = window; // Imports in UMD fashion.

const { directLine, store } = testHelpers.createDirectLineEmulator();

WebChat.renderWebChat({ directLine, store }, document.querySelector('main'));

await pageConditions.uiConnected();

const now = Date.now();
const timeForEachMessage = [];

for (let index = 0; index < NUM_MESSAGE; index++) {
const last = Date.now();

await directLine.emulateIncomingActivity(`Message ${index}.`);

timeForEachMessage.push(Date.now() - last);
}

await pageConditions.numActivitiesShown(NUM_MESSAGE);

console.log(`Took ${Date.now() - now} milliseconds.`);
console.log(timeForEachMessage.join(','));

// TODO: How do we consider performance don't exponentially increase?
});
</script>
</body>
</html>
@@ -1,6 +1,7 @@
import { hooks } from 'botframework-webchat-api';
import { useMemo } from 'react';

import type { WebChatActivity } from 'botframework-webchat-core';
import intersectionOf from '../../../Utils/intersectionOf';
import removeInline from '../../../Utils/removeInline';
import type { ActivityWithRenderer, ReadonlyActivityTree } from './types';
Expand Down Expand Up @@ -35,6 +36,10 @@ function validateAllEntriesTagged<T>(entries: readonly T[], bins: readonly (read

function useActivityTreeWithRenderer(entries: readonly ActivityWithRenderer[]): ReadonlyActivityTree {
const groupActivities = useGroupActivities();
const entryMap: Map<WebChatActivity, ActivityWithRenderer> = useMemo(
() => new Map(entries.map(entry => [entry.activity, entry])),
[entries]
);

// We bin activities in 2 different ways:
// - `activitiesBySender` is a 2D array containing activities with same sender
Expand All @@ -45,15 +50,15 @@ function useActivityTreeWithRenderer(entries: readonly ActivityWithRenderer[]):
entriesBySender: readonly (readonly ActivityWithRenderer[])[];
entriesByStatus: readonly (readonly ActivityWithRenderer[])[];
}>(() => {
const visibleActivities = entries.map(({ activity }) => activity);
const visibleActivities = [...entryMap.keys()];

const groupActivitiesResult = groupActivities({ activities: visibleActivities });

const activitiesBySender = groupActivitiesResult?.sender || [];
const activitiesByStatus = groupActivitiesResult?.status || [];

const [entriesBySender, entriesByStatus] = [activitiesBySender, activitiesByStatus].map(bins =>
bins.map(bin => bin.map(activity => entries.find(entry => entry.activity === activity)))
bins.map(bin => bin.map(activity => entryMap.get(activity)))
);

if (!validateAllEntriesTagged(visibleActivities, activitiesBySender)) {
Expand All @@ -72,7 +77,7 @@ function useActivityTreeWithRenderer(entries: readonly ActivityWithRenderer[]):
entriesBySender,
entriesByStatus
};
}, [entries, groupActivities]);
}, [entryMap, groupActivities]);

// Create a tree of activities with 2 dimensions: sender, followed by status.

Expand Down
3 changes: 0 additions & 3 deletions packages/test/harness/src/host/dev/hostOverrides/done.js
@@ -1,14 +1,11 @@
// In dev mode, draw a green tick when test succeeded.

const checkAccessibility = require('../../common/host/checkAccessibility');
const dumpLogs = require('../../common/dumpLogs');
const override = require('../utils/override');

// Send the completion back to the browser console.
module.exports = (webDriver, done) =>
override(done, undefined, async () => {
await checkAccessibility(webDriver)();

/* istanbul ignore next */
await webDriver.executeScript(() => {
console.log(
Expand Down
@@ -1,11 +1,11 @@
import createDeferred from 'p-defer';
import Observable from 'core-js/features/observable';
import random from 'math-random';
import createDeferred from 'p-defer';
import updateIn from 'simple-update-in';

import { createStoreWithOptions } from './createStore';
import became from '../pageConditions/became';
import createDeferredObservable from '../../utils/createDeferredObservable';
import became from '../pageConditions/became';
import { createStoreWithOptions } from './createStore';
import shareObservable from './shareObservable';

function isNativeClock() {
Expand Down Expand Up @@ -119,7 +119,7 @@ export default function createDirectLineEmulator({ autoConnect = true, ponyfill
};
},
emulateConnected: connectedDeferred.resolve,
emulateIncomingActivity: async activity => {
emulateIncomingActivity: async (activity, { skipWait } = {}) => {
if (typeof activity === 'string') {
activity = {
from: { id: 'bot', role: 'bot' },
Expand All @@ -145,11 +145,12 @@ export default function createDirectLineEmulator({ autoConnect = true, ponyfill

activityDeferredObservable.next(activity);

await became(
'incoming activity appears in the store',
() => store.getState().activities.find(activity => activity.id === id),
1000
);
skipWait ||
(await became(
'incoming activity appears in the store',
() => store.getState().activities.find(activity => activity.id === id),
1000
));
},
emulateOutgoingActivity: (activity, options) => {
if (typeof activity === 'string') {
Expand Down

0 comments on commit e44dd95

Please sign in to comment.