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

[Perf] Improved performance for useActivityTreeWithRenderer #5163

Merged
merged 14 commits into from
May 8, 2024
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
// In dev mode, draw a green tick when test succeeded.

const checkAccessibility = require('../../common/host/checkAccessibility');
compulim marked this conversation as resolved.
Show resolved Hide resolved
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)();
compulim marked this conversation as resolved.
Show resolved Hide resolved

/* istanbul ignore next */
await webDriver.executeScript(() => {
console.log(
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Loading