Skip to content

Commit

Permalink
feat(gatsby): use V8.serialize instead of JSON.stringify if available (
Browse files Browse the repository at this point in the history
…#10732)

Use `v8.serialize` instead of `json-stringify-safe` (if available) for persisting redux state to fix OOM issues and small performance gain (from ~250ms to ~150ms in the markdown benchmark).
  • Loading branch information
stefanprobst authored and pieh committed Apr 11, 2019
1 parent e010778 commit c043816
Show file tree
Hide file tree
Showing 8 changed files with 308 additions and 84 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`track root nodes Node sanitization Remove not supported fields / values 1`] = `
Object {
"children": Array [],
"id": "id1",
"inlineArray": Array [
1,
2,
3,
],
"inlineObject": Object {
"field": "fieldOfFirstNode",
},
"internal": Object {
"contentDigest": "digest1",
"owner": "test",
"type": "Test",
},
"parent": null,
}
`;
72 changes: 71 additions & 1 deletion packages/gatsby/src/db/__tests__/node-tracking-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ const {
boundActionCreators: { createNode },
} = require(`../../redux/actions`)
const { getNode } = require(`../../db/nodes`)
const { findRootNodeAncestor, trackDbNodes } = require(`../node-tracking`)
const {
findRootNodeAncestor,
trackDbNodes,
trackInlineObjectsInRootNode,
} = require(`../node-tracking`)
const { run: runQuery } = require(`../nodes-query`)
require(`./fixtures/ensure-loki`)()

Expand Down Expand Up @@ -136,4 +140,70 @@ describe(`track root nodes`, () => {
expect(findRootNodeAncestor(result[0].inlineObject)).toEqual(result[0])
})
})

describe(`Node sanitization`, () => {
let testNode
beforeEach(() => {
testNode = {
id: `id1`,
parent: null,
children: [],
unsupported: () => {},
inlineObject: {
field: `fieldOfFirstNode`,
re: /re/,
},
inlineArray: [1, 2, 3, Symbol(`test`)],
internal: {
type: `Test`,
contentDigest: `digest1`,
owner: `test`,
},
}
})

it(`Remove not supported fields / values`, () => {
const result = trackInlineObjectsInRootNode(testNode, true)
expect(result).toMatchSnapshot()
expect(result.unsupported).not.toBeDefined()
expect(result.inlineObject.re).not.toBeDefined()
expect(result.inlineArray[3]).not.toBeDefined()
})

it(`Doesn't mutate original`, () => {
trackInlineObjectsInRootNode(testNode, true)
expect(testNode.unsupported).toBeDefined()
expect(testNode.inlineObject.re).toBeDefined()
expect(testNode.inlineArray[3]).toBeDefined()
})

it(`Create copy of node if it has to remove anything`, () => {
const result = trackInlineObjectsInRootNode(testNode, true)
expect(result).not.toBe(testNode)
})

it(`Doesn't create clones if it doesn't have to`, () => {
const testNodeWithoutUnserializableData = {
id: `id1`,
parent: null,
children: [],
inlineObject: {
field: `fieldOfFirstNode`,
},
inlineArray: [1, 2, 3],
internal: {
type: `Test`,
contentDigest: `digest1`,
owner: `test`,
},
}

const result = trackInlineObjectsInRootNode(
testNodeWithoutUnserializableData,
true
)
// should be same instance
expect(result).toBe(testNodeWithoutUnserializableData)
})
})
})
90 changes: 75 additions & 15 deletions packages/gatsby/src/db/node-tracking.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,36 +9,96 @@ const rootNodeMap = new WeakMap()

const getRootNodeId = node => rootNodeMap.get(node)

/**
* @param {Object} data
* @returns {Object} data without undefined values
*/
const omitUndefined = data => {
const isPlainObject = _.isPlainObject(data)
if (isPlainObject) {
return _.pickBy(data, p => p !== undefined)
}

return data.filter(p => p !== undefined)
}

/**
* @param {*} data
* @return {boolean}
*/
const isTypeSupported = data => {
if (data === null) {
return true
}

const type = typeof data
const isSupported =
type === `number` ||
type === `string` ||
type === `boolean` ||
data instanceof Date

return isSupported
}

/**
* Add link between passed data and Node. This function shouldn't be used
* directly. Use higher level `trackInlineObjectsInRootNode`
* @see trackInlineObjectsInRootNode
* @param {(Object|Array)} data Inline object or array
* @param {string} nodeId Id of node that contains data passed in first parameter
* @param {boolean} sanitize Wether to strip objects of unuspported and not serializable fields
* @param {string} [ignore] Fieldname that doesn't need to be tracked and sanitized
*
*/
const addRootNodeToInlineObject = (data, nodeId) => {
if (_.isPlainObject(data) || _.isArray(data)) {
_.each(data, o => addRootNodeToInlineObject(o, nodeId))
rootNodeMap.set(data, nodeId)
const addRootNodeToInlineObject = (data, nodeId, sanitize, isNode = false) => {
const isPlainObject = _.isPlainObject(data)

if (isPlainObject || _.isArray(data)) {
let returnData = data
if (sanitize) {
returnData = isPlainObject ? {} : []
}
let anyFieldChanged = false
_.each(data, (o, key) => {
if (isNode && key === `internal`) {
returnData[key] = o
return
}
returnData[key] = addRootNodeToInlineObject(o, nodeId, sanitize)

if (returnData[key] !== o) {
anyFieldChanged = true
}
})

if (anyFieldChanged) {
data = omitUndefined(returnData)
}

// don't need to track node itself
if (!isNode) {
rootNodeMap.set(data, nodeId)
}

// arrays and plain objects are supported - no need to to sanitize
return data
}

if (sanitize && !isTypeSupported(data)) {
return undefined
}
// either supported or not sanitizing
return data
}

/**
* Adds link between inline objects/arrays contained in Node object
* and that Node object.
* @param {Node} node Root Node
*/
const trackInlineObjectsInRootNode = node => {
_.each(node, (v, k) => {
// Ignore the node internal object.
if (k === `internal`) {
return
}
addRootNodeToInlineObject(v, node.id)
})

return node
}
const trackInlineObjectsInRootNode = (node, sanitize = false) =>
addRootNodeToInlineObject(node, node.id, sanitize, true)
exports.trackInlineObjectsInRootNode = trackInlineObjectsInRootNode

/**
Expand Down
25 changes: 25 additions & 0 deletions packages/gatsby/src/redux/__tests__/__snapshots__/index.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`redux db should write cache to disk 1`] = `
Object {
"componentDataDependencies": Object {
"connections": Object {},
"nodes": Object {},
},
"components": Map {
"/Users/username/dev/site/src/templates/my-sweet-new-page.js" => Object {
"componentPath": "/Users/username/dev/site/src/templates/my-sweet-new-page.js",
"isInBootstrap": true,
"pages": Array [
"/my-sweet-new-page/",
],
"query": "",
},
},
"jsonDataPaths": Object {},
"staticQueryComponents": Map {},
"status": Object {
"plugins": Object {},
},
}
`;
54 changes: 43 additions & 11 deletions packages/gatsby/src/redux/__tests__/index.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,32 @@
const path = require(`path`)
const fs = require(`fs-extra`)
const { saveState, store } = require(`../index`)
const _ = require(`lodash`)

const writeToCache = jest.spyOn(require(`../persist`), `writeToCache`)
const { saveState, store, readState } = require(`../index`)

const {
actions: { createPage },
} = require(`../actions`)

jest.mock(`fs-extra`)
const mockWrittenContent = new Map()
jest.mock(`fs-extra`, () => {
return {
writeFileSync: jest.fn((file, content) =>
mockWrittenContent.set(file, content)
),
readFileSync: jest.fn(file => mockWrittenContent.get(file)),
}
})

describe(`redux db`, () => {
const initialComponentsState = _.cloneDeep(store.getState().components)

beforeEach(() => {
store.dispatch(
createPage(
{
path: `/my-sweet-new-page/`,
component: path.resolve(`./src/templates/my-sweet-new-page.js`),
// seems like jest serializer doesn't play nice with Maps on Windows
component: `/Users/username/dev/site/src/templates/my-sweet-new-page.js`,
// The context is passed as props to the component as well
// as into the component's GraphQL query.
context: {
Expand All @@ -24,23 +37,42 @@ describe(`redux db`, () => {
)
)

fs.writeFile.mockClear()
writeToCache.mockClear()
mockWrittenContent.clear()
})

it(`expect components state to be empty initially`, () => {
expect(initialComponentsState).toEqual(new Map())
})

it(`should write cache to disk`, async () => {
await saveState()

expect(fs.writeFile).toBeCalledWith(
expect.stringContaining(`.cache/redux-state.json`),
expect.stringContaining(`my-sweet-new-page.js`)
)
expect(writeToCache).toBeCalled()

// reset state in memory
store.dispatch({
type: `DELETE_CACHE`,
})
// make sure store in memory is empty
expect(store.getState().components).toEqual(initialComponentsState)

// read data that was previously cached
const data = readState()

// make sure data was read and is not the same as our clean redux state
expect(data.components).not.toEqual(initialComponentsState)

// yuck - loki and redux will have different shape of redux state (nodes and nodesByType)
expect(_.omit(data, [`nodes`, `nodesByType`])).toMatchSnapshot()
})

it(`does not write to the cache when DANGEROUSLY_DISABLE_OOM is set`, async () => {
process.env.DANGEROUSLY_DISABLE_OOM = true

await saveState()

expect(fs.writeFile).not.toBeCalled()
expect(writeToCache).not.toBeCalled()

delete process.env.DANGEROUSLY_DISABLE_OOM
})
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby/src/redux/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ actions.createNode = (
)
}

trackInlineObjectsInRootNode(node)
node = trackInlineObjectsInRootNode(node, true)

const oldNode = getNode(node.id)

Expand Down
Loading

0 comments on commit c043816

Please sign in to comment.