Skip to content

Commit

Permalink
perf: move id: eq fast path handling to node-model so it's shared bet…
Browse files Browse the repository at this point in the history
…ween query running strategies (gatsbyjs#34520)
  • Loading branch information
pieh authored and wardpeet committed Jan 19, 2022
1 parent 36e36a6 commit 728d156
Show file tree
Hide file tree
Showing 3 changed files with 190 additions and 25 deletions.
24 changes: 0 additions & 24 deletions packages/gatsby/src/datastore/lmdb/query/run-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { IGatsbyNode } from "../../../redux/types"
import { GatsbyIterable } from "../../common/iterable"
import {
createDbQueriesFromObject,
DbComparator,
DbQuery,
dbQueryToDottedField,
getFilterStatement,
Expand Down Expand Up @@ -54,16 +53,6 @@ export async function doRunQuery(args: IDoRunQueryArgs): Promise<IQueryResult> {
// Note: Keeping doRunQuery method the only async method in chain for perf
const context = createQueryContext(args)

// Fast-path: filter by node id
const nodeId = getFilterById(context)
if (nodeId) {
const node = args.datastore.getNode(nodeId)
return {
entries: new GatsbyIterable(node ? [node] : []),
totalCount: async (): Promise<number> => (node ? 1 : 0),
}
}

const totalCount = async (): Promise<number> =>
runCountOnce({ ...context, limit: undefined, skip: 0 })

Expand Down Expand Up @@ -333,19 +322,6 @@ function isFullyFiltered(
return dbQueries.length === usedQueries.size
}

function getFilterById(context: IQueryContext): string | undefined {
for (const q of context.dbQueries) {
const filter = getFilterStatement(q)
if (
filter.comparator === DbComparator.EQ &&
dbQueryToDottedField(q) === `id`
) {
return String(filter.value)
}
}
return undefined
}

function createNodeSortComparator(sortFields: SortFields): (a, b) => number {
const resolvedNodesCache = store.getState().resolvedNodesCache

Expand Down
153 changes: 153 additions & 0 deletions packages/gatsby/src/schema/__tests__/queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const apiRunnerNode = require(`../../utils/api-runner-node`)

const nodes = require(`./fixtures/queries`)

const { getDataStore } = require(`../../datastore`)

jest.mock(`gatsby-cli/lib/reporter`, () => {
return {
log: jest.fn(),
Expand Down Expand Up @@ -2011,4 +2013,155 @@ describe(`Query schema`, () => {
`)
})
})

describe(`id.eq fast path`, () => {
let datastoreRunQuerySpy
beforeAll(() => {
datastoreRunQuerySpy = jest.spyOn(getDataStore(), `runQuery`)
})

beforeEach(() => {
datastoreRunQuerySpy.mockClear()
})

afterAll(() => {
datastoreRunQuerySpy.mockRestore()
})

const queryEqId = `
query($id: String!) {
markdown(id: { eq: $id }) {
frontmatter {
title
}
}
}
`

it(`skips running datastore runQuery (there is node that satisfies filters)`, async () => {
const results = await runQuery(queryEqId, { id: `md2` })
expect(results).toMatchInlineSnapshot(`
Object {
"data": Object {
"markdown": Object {
"frontmatter": Object {
"title": "Markdown File 2",
},
},
},
}
`)
expect(datastoreRunQuerySpy).toBeCalledTimes(0)
})

it(`skips running datastore runQuery (there is no node that satisfies filters)`, async () => {
const results = await runQuery(queryEqId, { id: `that-should-not-exist` })
expect(results).toMatchInlineSnapshot(`
Object {
"data": Object {
"markdown": null,
},
}
`)
expect(datastoreRunQuerySpy).toBeCalledTimes(0)
})

it(`respect node type`, async () => {
const id = `file2`

{
const results = await runQuery(queryEqId, { id })
expect(results).toMatchInlineSnapshot(`
Object {
"data": Object {
"markdown": null,
},
}
`)
expect(datastoreRunQuerySpy).toBeCalledTimes(0)
}

{
// we didn't find a node above, but let's make sure there is a node with given id
const results = await runQuery(
`
query($id: String!) {
file(id: { eq: $id }) {
name
}
}
`,
{
id,
}
)
expect(results).toMatchInlineSnapshot(`
Object {
"data": Object {
"file": Object {
"name": "2.md",
},
},
}
`)
expect(datastoreRunQuerySpy).toBeCalledTimes(0)
}
})

describe(`doesn't try to use fast path if there are more or different filters than just id.eq`, () => {
it(`using single filter different than id.eq`, async () => {
const results = await runQuery(
`
query($title: String!) {
markdown(frontmatter: { title: { eq: $title } }) {
frontmatter {
title
}
}
}
`,
{ title: `Markdown File 2` }
)
expect(results).toMatchInlineSnapshot(`
Object {
"data": Object {
"markdown": Object {
"frontmatter": Object {
"title": "Markdown File 2",
},
},
},
}
`)
expect(datastoreRunQuerySpy).toBeCalledTimes(1)
})
})

it(`using multiple filters `, async () => {
const results = await runQuery(
`
query($id: String!, $title: String!) {
markdown(id: { eq: $id }, frontmatter: { title: { eq: $title } }) {
frontmatter {
title
}
}
}
`,
{ title: `Markdown File 2`, id: `md2` }
)
expect(results).toMatchInlineSnapshot(`
Object {
"data": Object {
"markdown": Object {
"frontmatter": Object {
"title": "Markdown File 2",
},
},
},
}
`)
expect(datastoreRunQuerySpy).toBeCalledTimes(1)
})
})
})
38 changes: 37 additions & 1 deletion packages/gatsby/src/schema/node-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,42 @@ class LocalNodeModel {

const nodeTypeNames = toNodeTypeNames(this.schema, gqlType)

let runQueryActivity

// check if we can get node by id and skip
// more expensive query pipeline
if (
typeof query?.filter?.id?.eq !== `undefined` &&
Object.keys(query.filter).length === 1 &&
Object.keys(query.filter.id).length === 1
) {
if (tracer) {
runQueryActivity = reporter.phantomActivity(`runQuerySimpleIdEq`, {
parentSpan: tracer.getParentActivity().span,
})
runQueryActivity.start()
}
let nodeFoundById = getDataStore().getNode(query.filter.id.eq)

// make sure our node is of compatible type
if (
nodeFoundById &&
!nodeTypeNames.includes(nodeFoundById.internal.type)
) {
nodeFoundById = null
}

if (runQueryActivity) {
runQueryActivity.end()
}

return {
gqlType,
entries: new GatsbyIterable(nodeFoundById ? [nodeFoundById] : []),
totalCount: async () => (nodeFoundById ? 1 : 0),
}
}

let materializationActivity
if (tracer) {
materializationActivity = reporter.phantomActivity(`Materialization`, {
Expand All @@ -283,6 +319,7 @@ class LocalNodeModel {
min: query.min,
sum: query.sum,
})

const fieldsToResolve = determineResolvableFields(
this.schemaComposer,
this.schema,
Expand All @@ -300,7 +337,6 @@ class LocalNodeModel {
materializationActivity.end()
}

let runQueryActivity
if (tracer) {
runQueryActivity = reporter.phantomActivity(`runQuery`, {
parentSpan: tracer.getParentActivity().span,
Expand Down

0 comments on commit 728d156

Please sign in to comment.