perf(query): skip recursive casting for simple equality filters #15754
Conversation
|
@vkarpov15 can you approve this please. |
| } | ||
| function _isSimpleEqualityFilter(filter, model) { | ||
| if (!filter || typeof filter !== 'object' || Array.isArray(filter)) return false; | ||
| const keys = Object.keys(filter); |
There was a problem hiding this comment.
isn't that expensive to create an array instead of a for loop ?
There was a problem hiding this comment.
@billouboq
This approach is crucial for correctly validating the filter structure because it offers two key safety guarantees:
It guarantees iteration only over the object's own enumerable properties.
It automatically excludes inherited properties (which a standard for...in loop would include without an extra hasOwnProperty check).
vkarpov15
left a comment
vkarpov15
left a comment
There was a problem hiding this comment.
This PR skips all casting if you have only string/number/boolean properties in your filter, which is incorrect behavior.
Can you please put together a benchmark script which demonstrates a case where this change would make for a significant performance improvement?
|
|
||
| const t = typeof v; | ||
| if (t === 'string' || t === 'number' || t === 'boolean' || v instanceof RegExp) { | ||
| const schemaType = model && model.schema ? model.schema.path(k) : null; |
There was a problem hiding this comment.
This doesn't check if the t lines up with schemaType. If you pass in email: 42 this will skip casting number 42 to a string.
| const t = typeof v; | ||
| if (t === 'string' || t === 'number' || t === 'boolean' || v instanceof RegExp) { | ||
| const schemaType = model && model.schema ? model.schema.path(k) : null; | ||
| if (schemaType && schemaType.instance && /ObjectId/i.test(schemaType.instance)) return false; |
There was a problem hiding this comment.
Also wouldn't handle if schemaType is an object, array, or map.
|
I'm going to close this PR since I don't think it is salvageable. |
3 participants
Summary
This PR introduces a conservative performance optimization in
Query.prototype._castConditions.For simple equality filters (e.g.
{ name: "Alice" },{ age: 20 }), Mongoose currently performs the full recursive casting pipeline, which adds noticeable overhead compared to the native MongoDB driver.This PR adds a fast-path that skips recursive casting when the filter meets all of the following conditions:
$...)string,number,boolean),RegExp, ornullIf all conditions are satisfied,
_castConditionsreturns early and avoids the expensive casting process.Motivation
Mongoose users have reported slower query performance compared to the native driver, especially for frequently-used simple lookups such as: