Repository navigation
Conversation
…t (O(n*m) -> O(n+m)) Each upsert loop re-scanned the existing fields/filter groups/filters/sorts with .find() for every input item. Build the lookup maps once (first occurrence wins, matching the previous .find() semantics) and look up in O(1).
…n*m) -> O(n+m)) processNestedRelations re-scanned the whole relationResults array for every parent record — .filter() for ONE_TO_MANY and .find() otherwise. Index the rows once (grouped by join field, or by id with first-occurrence-wins to match .find) and look up in O(1).
|
👋 Thanks for contributing to Twenty! We're excited to have you on board. Your PR has been set to draft while you work on it. Once you're done, mark it as Ready for review and our automated checks will run. By submitting your Pull Request, you acknowledge that you agree with the terms of our Contributor License Agreement. |
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
etiennejouan
left a comment
There was a problem hiding this comment.
Hi @yoo-minho, thank you for your contribution. I think the performance gains here are not substantial enough to justify merging this PR. I've left you some comments about Twenty patterns and our codebase style.
Please feel free to tackle any open issues, I would be happy to review and merge !
| // Index the relation rows once instead of re-scanning them for every parent | ||
| // record (O(n*m) -> O(n+m)). Parent ids are unique, so each bucket belongs to |
There was a problem hiding this comment.
No comment if avoidable in twenty codebase
| // Index the relation rows once instead of re-scanning them for every parent | ||
| // record (O(n*m) -> O(n+m)). Parent ids are unique, so each bucket belongs to | ||
| // exactly one parent. | ||
| // oxlint-disable-next-line typescript/no-explicit-any |
There was a problem hiding this comment.
Exceptions like this are real code smell. It's really rare and you should always wonder if there is no other way
| for (const rel of relationResults) { | ||
| const bucket = relationsByJoinField.get(rel[joinField]); | ||
|
|
||
| if (bucket) { |
What
processNestedRelationshydrates relations onto parent records, and for every parent record it re-scans the entirerelationResultsarray:That is
O(n*m)— parent records × relation rows — on every query that selects a relation. A page of 60 records with 500 relation rows is ~30k comparisons, and it grows with both page size and relation cardinality.How
Index
relationResultsonce before the loop, then look up inO(1):joinFieldintoMap<key, rel[]>, thenmap.get(item.id) ?? [](the?? []preserves the empty-array result of.filter).id, first occurrence wins so the behaviour matchesArray.prototype.find(a plainnew Map(arr.map(...))would keep the last one).Parent ids are unique, so each
ONE_TO_MANYbucket belongs to exactly one parent — no array is shared between records.No behaviour change; only the lookup strategy.
Found with cleanscore (flags linear scans of an outer array inside a loop) and verified by hand. Description drafted with LLM assistance.