Skip to content

Commit

Permalink
perf: partially lift matching from regexp to js (#9032)
Browse files Browse the repository at this point in the history
Digging further into #3857.

See also #8955, #8956.

As [previously
discussed](#3857 (comment)),
the query builder currently suffers from poor performance in two ways:
quadratic numbers of operations with respect to total table/column
counts, and poor constant factor performance (regexps can be expensive
to build/run!)

The constant-factor performance is the more tractable problem: no longer
quadratically looping would be a chunky rewrite of the query builder,
but we can locally refactor to be a bunch cheaper in terms of regexp
operations.

This change cuts the benchmark time here in ~half (yay!).

We achieve this by simplifying the overall replacement regexp (we don't
need our column names in there, since we already have a plain object
where they're the keys to match against) so compilation of that is much
cheaper, plus skipping the need to `escapeRegExp` every column as a
result.
  • Loading branch information
draaglom authored May 31, 2022
1 parent 862a402 commit bbdc20f
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 25 deletions.
40 changes: 20 additions & 20 deletions src/query-builder/QueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -756,27 +756,27 @@ export abstract class QueryBuilder<Entity> {
replacements[column.propertyPath] = column.databaseName
}

const replacementKeys = Object.keys(replacements)

if (replacementKeys.length) {
statement = statement.replace(
new RegExp(
// Avoid a lookbehind here since it's not well supported
`([ =\(]|^.{0})` +
`${escapeRegExp(
replaceAliasNamePrefix,
)}(${replacementKeys
.map(escapeRegExp)
.join("|")})` +
`(?=[ =\)\,]|.{0}$)`,
"gm",
),
(_, pre, p) =>
`${pre}${replacementAliasNamePrefix}${this.escape(
statement = statement.replace(
new RegExp(
// Avoid a lookbehind here since it's not well supported
`([ =\(]|^.{0})` + // any of ' =(' or start of line
// followed by our prefix, e.g. 'tablename.' or ''
`${escapeRegExp(
replaceAliasNamePrefix,
)}([^ =\(\)\,]+)` + // a possible property name: sequence of anything but ' =(),'
// terminated by ' =),' or end of line
`(?=[ =\)\,]|.{0}$)`,
"gm",
),
(match, pre, p) => {
if (replacements[p]) {
return `${pre}${replacementAliasNamePrefix}${this.escape(
replacements[p],
)}`,
)
}
)}`
}
return match
},
)
}

return statement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ describe("benchmark > QueryBuilder > wide join", () => {

/**
* On a M1 macbook air, 5 runs:
* 3501ms
* 3574ms
* 3575ms
* 3563ms
* 3567ms
* 1861ms
* 1850ms
* 1859ms
* 1859ms
* 1884ms
*/
})
})

0 comments on commit bbdc20f

Please sign in to comment.