Skip to content

Commit e69b274

Browse files
committed
perf: run mandatory populate logic once per querymap
Instead of running it for each dataloaderFind we now run it once for each queryMap. We dont add any more overhead to getNewFiltersAndMapKeys but now we have to fully traverse newFilter one additional time just to compute the mandatory populate options.
1 parent 2ac3bc0 commit e69b274

File tree

3 files changed

+35
-57
lines changed

3 files changed

+35
-57
lines changed

.changeset/afraid-cars-attend.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"mikro-orm-find-dataloader": minor
3+
---
4+
5+
perf: run mandatory populate logic once per querymap

packages/find/src/find.test.ts

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -124,23 +124,9 @@ describe("find", () => {
124124
it("should fetch books with the find dataloader", async () => {
125125
const authors = await em.fork().find(Author, {});
126126
const mock = mockLogger(orm);
127-
const authorBooks = await Promise.all([
128-
...authors.map(async ({ id }) => await em.getRepository(Book).find({ author: id })),
129-
// em.getRepository(Book).find({ author: { books: { author: 1 } } }),
130-
// em.getRepository(Book).find({ title: "a", author: [1, { books: { author: 1 } }] }),
131-
// em.getRepository(Book).find({ title: "a", author: { books: { author: 1 } } }),
132-
// em.getRepository(Book).find({ title: "a", author: { books: { author: { name: "a" } } } }),
133-
// em.getRepository(Book).find({ title: "a", author: { books: { title: "a" } } }),
134-
// em.getRepository(Book).find({ title: "a", author: { books: 1 } }),
135-
// em.getRepository(Book).find({ author: 1 }),
136-
// em.getRepository(Book).find({ author: 1 }),
137-
// em.getRepository(Book).find({ id: 2, title: "b", author: { id: 1, name: "a" } }),
138-
// em.getRepository(Book).find({ author: { id: 1, name: "a" } }),
139-
// em.getRepository(Book).find({ author: { id: 2 } }),
140-
// em.getRepository(Book).find({ author: { id: 3 } }),
141-
// em.getRepository(Book).find({ author: { name: "a" } }),
142-
]);
143-
// console.log(mock.mock.calls);
127+
const authorBooks = await Promise.all(
128+
authors.map(async ({ id }) => await em.getRepository(Book).find({ author: id })),
129+
);
144130
expect(authorBooks).toBeDefined();
145131
expect(authorBooks).toMatchSnapshot();
146132
expect(mock.mock.calls).toEqual([

packages/find/src/findDataloader.ts

Lines changed: 27 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -263,20 +263,20 @@ COMPOSITE PK:
263263
const asc = (a: string, b: string): number => a.localeCompare(b);
264264
const notNull = (el: string | undefined): boolean => el != null;
265265

266-
function getNewFiltersMapKeysAndMandatoryPopulate<K extends object>(
266+
function getNewFiltersAndMapKeys<K extends object>(
267267
cur: FilterQueryDataloader<K>,
268268
meta: EntityMetadata<K>,
269269
entityName: string,
270-
): Array<[FilterQueryDataloader<K>, string, Set<string>?]>;
271-
function getNewFiltersMapKeysAndMandatoryPopulate<K extends object>(
270+
): Array<[FilterQueryDataloader<K>, string]>;
271+
function getNewFiltersAndMapKeys<K extends object>(
272272
cur: FilterQueryDataloader<K>,
273273
meta: EntityMetadata<K>,
274-
): [FilterQueryDataloader<K>, string, string?];
275-
function getNewFiltersMapKeysAndMandatoryPopulate<K extends object>(
274+
): [FilterQueryDataloader<K>, string];
275+
function getNewFiltersAndMapKeys<K extends object>(
276276
cur: FilterQueryDataloader<K>,
277277
meta: EntityMetadata<K>,
278278
entityName?: string,
279-
): [FilterQueryDataloader<K>, string, string?] | Array<[FilterQueryDataloader<K>, string, Set<string>?]> {
279+
): [FilterQueryDataloader<K>, string] | Array<[FilterQueryDataloader<K>, string]> {
280280
const PKs = getPKs(cur, meta);
281281
if (PKs != null) {
282282
const res: [FilterQueryDataloader<K>, string] = [
@@ -289,8 +289,6 @@ function getNewFiltersMapKeysAndMandatoryPopulate<K extends object>(
289289
} else {
290290
const newFilter: any = {};
291291
const keys: string[] = [];
292-
const populateSet = new Set<string>();
293-
let computedPopulate: string | undefined;
294292
if (Array.isArray(cur)) {
295293
// COMPOSITE PKs like [{owner: 1, recipient: 2}, {recipient: 4, owner: 3}]
296294
for (const key of meta.primaryKeys) {
@@ -308,42 +306,27 @@ function getNewFiltersMapKeysAndMandatoryPopulate<K extends object>(
308306
// Using $or at the top level means that we can treat it as two separate queries and filter results from either of them
309307
if (key === "$or" && entityName != null) {
310308
return (value as Array<FilterQueryDataloader<K>>)
311-
.map((el) => getNewFiltersMapKeysAndMandatoryPopulate(el, meta, entityName))
309+
.map((el) => getNewFiltersAndMapKeys(el, meta, entityName))
312310
.flat();
313311
}
314312
const keyProp = meta.properties[key as EntityKey<K>];
315313
if (keyProp == null) {
316314
throw new Error(`Cannot find properties for ${key}`);
317315
}
318316
if (keyProp.targetMeta == null) {
319-
// Our current key might lead to a scalar (thus we don't need to populate anything)
320-
// or to an explicited PK like {id: 1, name: 'a'}
321317
newFilter[key] = Array.isArray(value) ? value : [value];
322318
keys.push(key);
323319
} else {
324-
// Our current key points to either a Reference or a Collection
325-
const [subFilter, subKey, furtherPop] = getNewFiltersMapKeysAndMandatoryPopulate(value, keyProp.targetMeta);
320+
const [subFilter, subKey] = getNewFiltersAndMapKeys(value, keyProp.targetMeta);
326321
newFilter[key] = subFilter;
327322
keys.push(`${key}:${subKey}`);
328-
// We need to populate all Collections and all the References
329-
// where we further match non-PKs properties
330-
if (keyProp.ref !== true || !isPK(value, keyProp.targetMeta)) {
331-
computedPopulate = furtherPop == null ? `${key}` : `${key}.${furtherPop}`;
332-
if (entityName != null) {
333-
populateSet.add(computedPopulate);
334-
} else {
335-
// We return computedPopulate as the third element of the array
336-
}
337-
}
338323
}
339324
}
340325
const res: [FilterQueryDataloader<K>, string] = [
341326
newFilter,
342327
[entityName, `{${keys.sort(asc).join(",")}}`].filter(notNull).join("|"),
343328
];
344-
return entityName == null
345-
? [...res, computedPopulate]
346-
: [[...res, populateSet.size === 0 ? undefined : populateSet]];
329+
return entityName == null ? res : [res];
347330
}
348331
}
349332
}
@@ -386,19 +369,19 @@ function updateQueryFilter<K extends object, P extends string = never>(
386369
}
387370

388371
// The least amount of populate necessary to map the dataloader results to their original queries
389-
export function getMandatoryPopulate<K extends object>(
372+
function getMandatoryPopulate<K extends object>(
390373
cur: FilterQueryDataloader<K>,
391374
meta: EntityMetadata<K>,
392375
): string | undefined;
393-
export function getMandatoryPopulate<K extends object>(
376+
function getMandatoryPopulate<K extends object>(
394377
cur: FilterQueryDataloader<K>,
395378
meta: EntityMetadata<K>,
396-
populate: Set<any>,
379+
options: { populate?: Set<any> },
397380
): void;
398-
export function getMandatoryPopulate<K extends object>(
381+
function getMandatoryPopulate<K extends object>(
399382
cur: FilterQueryDataloader<K>,
400383
meta: EntityMetadata<K>,
401-
populate?: Set<any>,
384+
options?: { populate?: Set<any> },
402385
): any {
403386
for (const [key, value] of Object.entries(cur)) {
404387
const keyProp = meta.properties[key as EntityKey<K>];
@@ -410,12 +393,14 @@ export function getMandatoryPopulate<K extends object>(
410393
// Our current key points to either a Reference or a Collection
411394
// We need to populate all Collections
412395
// We also need to populate References whenever we have to further match non-PKs properties
413-
const PKs = getPKs(value, keyProp.targetMeta);
414-
if (keyProp.ref !== true || PKs == null) {
415-
const furtherPopulate = getMandatoryPopulate(value, keyProp.targetMeta);
416-
const computedPopulate = furtherPopulate == null ? `${key}` : `${key}.${furtherPopulate}`;
417-
if (populate != null) {
418-
populate.add(computedPopulate);
396+
if (keyProp.ref !== true || !isPK(value, keyProp.targetMeta)) {
397+
const furtherPop = getMandatoryPopulate(value, keyProp.targetMeta);
398+
const computedPopulate = furtherPop == null ? `${key}` : `${key}.${furtherPop}`;
399+
if (options != null) {
400+
if (options.populate == null) {
401+
options.populate = new Set();
402+
}
403+
options.populate.add(computedPopulate);
419404
} else {
420405
return computedPopulate;
421406
}
@@ -439,13 +424,15 @@ export function groupFindQueriesByOpts(
439424
const queriesMap = new Map<string, [FilterQueryDataloader<any>, { populate?: true | Set<any> }?]>();
440425
for (const dataloaderFind of dataloaderFinds) {
441426
const { entityName, meta, filter, options } = dataloaderFind;
442-
const filtersAndKeys = getNewFiltersMapKeysAndMandatoryPopulate(filter, meta, entityName);
427+
const filtersAndKeys = getNewFiltersAndMapKeys(filter, meta, entityName);
443428
dataloaderFind.filtersAndKeys = [];
444-
filtersAndKeys.forEach(([newFilter, key, mandatoryPopulate]) => {
429+
filtersAndKeys.forEach(([newFilter, key]) => {
445430
dataloaderFind.filtersAndKeys?.push({ key, newFilter });
446431
let queryMap = queriesMap.get(key);
447432
if (queryMap == null) {
448-
queryMap = [structuredClone(newFilter), { ...(mandatoryPopulate != null && { populate: mandatoryPopulate }) }];
433+
const queryMapOpts = {};
434+
queryMap = [structuredClone(newFilter), queryMapOpts];
435+
getMandatoryPopulate(newFilter, meta, queryMapOpts);
449436
updateQueryFilter(queryMap, newFilter, options, true);
450437
queriesMap.set(key, queryMap);
451438
} else {

0 commit comments

Comments
 (0)