perf: partially lift matching from regexp to js (#9032)

Digging further into #3857.

See also #8955, #8956.

As [previously
discussed](https://github.com/typeorm/typeorm/issues/3857#issuecomment-699505893),
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.
This commit is contained in:
Patrick Molgaard 2022-05-31 16:56:42 +01:00 committed by GitHub
parent 862a4027af
commit bbdc20f8ca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 25 deletions

View File

@ -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

View File

@ -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
*/
})
})