devs
/
emoji-picker-element
mirror of https://github.com/nolanlawson/emoji-picker-element.git
4
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix: fix and test memory leaks

This commit is contained in:
Nolan Lawson 2020-06-20 11:34:29 -07:00
parent 753fc274f2
commit 665a630913
12 changed files with 214 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
README.md
View File

@ -733,4 +733,8 @@ Benchmark memory usage:
Benchmark bundle size:
yarn benchmark:bundlesize
yarn benchmark:bundlesize
Run memory leak test:
yarn test:leak

3
package.json
View File

@ -26,6 +26,9 @@
"benchmark:memory:server": "node ./test/memory/server.js",
"benchmark:memory:test": "node ./test/memory/test.js",
"benchmark:run-bundlesize": "bundlesize",
"test:leak": "run-s build:rollup && run-p --race test:leak:server test:leak:test",
"test:leak:server": "node ./test/leak/server.js",
"test:leak:test": "node ./test/leak/test.js",
"dev": "run-p --race dev:rollup dev:server",
"dev:rollup": "NODE_ENV=development rollup -c -w",
"dev:server": "node ./test/adhoc/server.js",

8
src/picker/PickerElement.js
View File

@ -1,5 +1,6 @@
import SveltePicker from './components/Picker/Picker.svelte'
import { mark } from '../shared/marks'
import { log } from '../shared/log'
export default class Picker extends SveltePicker {
constructor (props) {
@ -7,6 +8,13 @@ export default class Picker extends SveltePicker {
// Make the API simpler, directly pass in the props
super({ props })
}
disconnectedCallback () {
// Have to explicitly destroy the component to avoid memory leaks.
// See https://github.com/sveltejs/svelte/issues/1152
log('disconnectedCallback')
this.$destroy()
}
}
customElements.define('emoji-picker', Picker)

15
src/picker/components/Picker/Picker.js
View File

@ -12,7 +12,6 @@ import { log } from '../../../shared/log'
import { applySkinTone } from '../../utils/applySkinTone'
import { halt } from '../../utils/halt'
import { incrementOrDecrement } from '../../utils/incrementOrDecrement'
import { tick } from 'svelte'
import {
DEFAULT_NUM_COLUMNS,
DEFAULT_SKIN_TONE_EMOJI, FONT_FAMILY,
@ -25,6 +24,7 @@ import { calculateWidth, resizeObserverSupported } from '../../utils/calculateWi
import { checkZwjSupport } from '../../utils/checkZwjSupport'
import { requestPostAnimationFrame } from '../../utils/requestPostAnimationFrame'
import { stop } from '../../../shared/marks'
import { onMount, onDestroy, tick } from 'svelte'
// public
let locale = null
@ -127,8 +127,10 @@ $: {
// renders custom elements in an odd way - props are not set when calling the constructor,
// but are only set later. This would cause a double render or a double-fetch of
// the dataSource, which is bad. Delaying with a microtask avoids this.
Promise.resolve().then(() => {
log('setting locale and dataSource to default')
// See https://github.com/sveltejs/svelte/pull/4527
onMount(async () => {
await tick()
log('props ready: setting locale and dataSource to default')
locale = locale || DEFAULT_LOCALE
dataSource = dataSource || DEFAULT_DATA_SOURCE
})
@ -139,6 +141,13 @@ $: {
}
}
onDestroy(async () => {
if (database) {
log('closing database')
await database.close()
}
})
//
// Global styles for the entire picker
//

20
test/leak/index.html Normal file
View File

@ -0,0 +1,20 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width">
</head>
<body>
<script type="module">
import Picker from '/picker.js'
window.addPicker = function addPicker() {
document.body.appendChild(new Picker({
dataSource: '/node_modules/emojibase-data/en/data.json'
}))
}
window.removePicker = function removePicker() {
document.body.removeChild(document.querySelector('emoji-picker'))
}
</script>
</body>
</html>

12
test/leak/server.js Normal file
View File

@ -0,0 +1,12 @@
import express from 'express'
import fs from 'fs'
const app = express()
const port = 3000
app.use(express.static('./'))
app.get('/', (req, res) => {
res.type('text/html').send(fs.readFileSync('./test/leak/index.html', 'utf8'))
})
app.listen(port, () => console.log(`Server running at http://localhost:${port}`))

124
test/leak/test.js Normal file
View File

@ -0,0 +1,124 @@
//
// Basic idea of this test is to add/remove the element to/from the DOM 10 times
// and then check for objects that are leaking some multiple of 10 times.
// I'd love to just say "if leaks > 0 then it's leaking", but Chrome seems to hold
// on to odd memory in odd places for no obvious reason.
//
/* global addPicker removePicker */
import puppeteer from 'puppeteer'
import fetch from 'node-fetch'
const ITERATIONS = 10
async function waitForServerReady () {
while (true) {
try {
const resp = await fetch('http://localhost:3000')
if (resp.status === 200) {
break
}
} catch (err) {}
console.log('Waiting for localhost:3000 to be available')
await new Promise(resolve => setTimeout(resolve, 1000))
}
}
// see https://addyosmani.com/blog/puppeteer-recipes/#measuring-memory-leaks
async function getObjects (page) {
const prototypeHandle = await page.evaluateHandle(() => Object.prototype)
const objectsHandle = await page.queryObjects(prototypeHandle)
const objectNames = await page.evaluate((instances) => instances.map(_ => (
`${_.constructor.name}::${typeof _}`
)), objectsHandle)
await Promise.all([
prototypeHandle.dispose(),
objectsHandle.dispose()
])
return objectNames
}
function objectsToCountMap (objects) {
const res = {}
for (const obj of objects) {
if (!res[obj]) {
res[obj] = 0
}
res[obj]++
}
return res
}
function diff (objectsBefore, objectsAfter) {
const countsBefore = objectsToCountMap(objectsBefore)
const countsAfter = objectsToCountMap(objectsAfter)
const diff = {}
for (const [obj, count] of Object.entries(countsAfter)) {
const beforeCount = countsBefore[obj] || 0
const diffCount = count - beforeCount
if (diffCount > 0) {
diff[obj] = diffCount
}
}
return diff
}
function sleep (ms) {
return new Promise(resolve => setTimeout(resolve, ms))
}
async function addAndRemovePicker (page) {
await page.evaluate(() => addPicker())
await sleep(1000)
await page.evaluate(() => removePicker())
await sleep(1000)
}
async function main () {
await waitForServerReady()
const browser = await puppeteer.launch({ headless: true })
const context = await browser.createIncognitoBrowserContext() // not sure why Addy uses incognito, but sure
const page = await context.newPage()
await page.goto('http://localhost:3000/')
console.log('Running', ITERATIONS, 'iterations...')
// run once to load any one-time JS costs
await addAndRemovePicker(page)
await sleep(5000)
const objectsBefore = await getObjects(page)
// do several iterations to identify obvious memory leaks (things leaking n times)
for (let i = 0; i < ITERATIONS; i++) {
console.log('iteration', i + 1)
await addAndRemovePicker(page)
}
await sleep(5000)
const objectsAfter = await getObjects(page)
await browser.close()
console.log('object count before', objectsBefore.length, 'object count after', objectsAfter.length)
const comparison = diff(objectsBefore, objectsAfter)
console.log('diff', comparison)
const likelyLeaks = [...Object.entries(comparison)]
.filter(([object, count]) => (count % ITERATIONS === 0))
if (likelyLeaks.length) {
console.log('Likely leaks', likelyLeaks)
throw new Error('Found likely leaks, throwing error')
} else {
console.log('No likely leaks')
}
}
main().catch(err => {
console.error(err)
process.exit(1)
})

1
test/memory/index.html
View File

@ -5,7 +5,6 @@
<meta name="viewport" content="width=device-width">
</head>
<body>
<script src="/node_modules/focus-visible/dist/focus-visible.js"></script>
<script>
(async () => {
const params = new URLSearchParams(location.search)

24
test/memory/test.js
View File

@ -10,6 +10,19 @@ const scenarios = [
'full'
]
async function waitForServerReady () {
while (true) {
try {
const resp = await fetch('http://localhost:3000')
if (resp.status === 200) {
break
}
} catch (err) {}
console.log('Waiting for localhost:3000 to be available')
await new Promise(resolve => setTimeout(resolve, 1000))
}
}
async function measureMemory (scenario) {
const browser = await puppeteer.launch({
headless: false, // required for performance.measureMemory()
@ -32,16 +45,7 @@ function printBytes (bytes) {
}
async function main () {
while (true) {
try {
const resp = await fetch('http://localhost:3000')
if (resp.status === 200) {
break
}
} catch (err) {}
console.log('Waiting for localhost:3000 to be availble')
await new Promise(resolve => setTimeout(resolve, 1000))
}
await waitForServerReady()
const results = []
for (const scenario of scenarios) {
const bytes = await measureMemory(scenario)

7
test/spec/picker/Picker.test.js
View File

@ -3,6 +3,7 @@ import * as testingLibrary from '@testing-library/dom'
import Picker from '../../../src/picker/PickerElement.js'
import userEvent from '@testing-library/user-event'
import { groups } from '../../../src/picker/groups'
import Database from '../../../src/database/Database'
const { waitFor, fireEvent } = testingLibrary
const { type } = userEvent
@ -33,10 +34,12 @@ describe('Picker tests', () => {
)
})
afterEach(async () => {
basicAfterEach()
await tick(20)
await picker.database.delete()
document.body.removeChild(picker)
await tick(20)
await new Database({ dataSource: ALL_EMOJI, locale: 'en' }).delete()
await tick(20)
basicAfterEach()
})
const numInGroup1 = truncatedEmoji.filter(_ => _.group === 0).length

6
test/spec/picker/element.test.js
View File

@ -35,11 +35,12 @@ describe('element tests', () => {
await tick(20)
})
afterEach(async () => {
document.body.removeChild(picker)
await tick(20)
await new Database({ dataSource: FR_EMOJI, locale: 'fr' }).delete()
await new Database({ dataSource: ALL_EMOJI, locale: 'en' }).delete()
await tick(20)
basicAfterEach()
document.body.removeChild(picker)
})
test('changing locale/dataSource causes only one network request', async () => {
@ -98,7 +99,8 @@ describe('element tests', () => {
await waitFor(() => expect(getByRole(container, 'menuitem', { name: /😀/ })).toBeVisible())
await picker.database.delete()
await new Database().delete()
await tick(20)
})
})
})

10
test/spec/picker/favorites.test.js
View File

@ -8,6 +8,9 @@ import allData from 'emojibase-data/en/data.json'
import { MOST_COMMONLY_USED_EMOJI } from '../../../src/picker/constants'
import { uniqBy } from '../../../src/shared/uniqBy'
import { groups } from '../../../src/picker/groups'
import Database from '../../../src/database/Database'
const dataSource = 'with-favs.json'
describe('Favorites UI', () => {
let picker
@ -15,7 +18,6 @@ describe('Favorites UI', () => {
beforeEach(async () => {
basicBeforeEach()
const dataSource = 'with-favs.json'
const dataWithFavorites = uniqBy([
...truncatedEmoji,
@ -32,10 +34,12 @@ describe('Favorites UI', () => {
await tick(20)
})
afterEach(async () => {
basicAfterEach()
await tick(20)
await picker.database.delete()
document.body.removeChild(picker)
await tick(20)
await new Database({ dataSource, locale: 'en' }).delete()
await tick(20)
basicAfterEach()
})
test('Favorites UI basic test', async () => {