Skip to content

Commit

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

See also typeorm#8955, typeorm#8956.

As [previously
discussed](typeorm#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 and frangz committed Nov 11, 2022
1 parent f53e2f9 commit d494f3a
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 17 deletions.
33 changes: 21 additions & 12 deletions src/query-builder/QueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -617,18 +617,27 @@ export abstract class QueryBuilder<Entity> {
replacements[column.propertyPath] = column.databaseName;
}

const replacementKeys = Object.keys(replacements);

if (replacementKeys.length) {
statement = statement.replace(new RegExp(
`(?<=[ =\(]|^.{0})` +
`${escapeRegExp(replaceAliasNamePrefix)}(${replacementKeys.map(escapeRegExp).join("|")})` +
`(?=[ =\)\,]|.{0}$)`,
"gm"
), (_, p) =>
`${replacementAliasNamePrefix}${this.escape(replacements[p])}`
);
}
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 d494f3a

Please sign in to comment.