optimize: add a descendant map to speed up v5 export performance#8805
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a descendant‐type map to optimize withDescendants lookups during v5 exports, reducing unnecessary queries and improving export performance.
- Support passing a
queryTypesDescendantMaptodatabase.withDescendants - Declare the export‐type descendant relationships in
WORKSPACE_EXPORT_TYPES_DESCENDANT_MAP - Update export logic and tests to leverage the new map
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/insomnia/src/models/index.ts | Added WORKSPACE_EXPORT_TYPES_DESCENDANT_MAP constant |
| packages/insomnia/src/common/insomnia-v5.ts | Passed the descendant map into withDescendants in the export flow |
| packages/insomnia/src/common/database.ts | Extended withDescendants to accept and apply a descendant‐type map |
| packages/insomnia/src/common/tests/database.test.ts | Added tests covering the new descendant‐map behavior |
Comments suppressed due to low confidence (3)
packages/insomnia/src/common/database.ts:621
- Consider adding a JSDoc comment for
queryTypesDescendantMapto explain its purpose, expected key format, and fallback behavior when a type is missing from the map.
queryTypesDescendantMap?: Record<string, string[]>,
packages/insomnia/src/common/insomnia-v5.ts:457
- The
workspaceDescendantsvariable is assigned but never used—either use it to filter export items or remove this assignment to avoid dead code.
const workspaceDescendants = await database.withDescendants(
packages/insomnia/src/common/database.ts:641
- The order of
typesdetermines traversal order and can lead to nondeterministic result ordering; consider sorting or explicitly defining type order to ensure consistent descendant lists.
const types = queryTypesFromDescendantMap ?? (queryTypes?.length ? queryTypes : allTypes());
| [EXPORT_TYPE_PROTO_DIRECTORY]: protoDirectory, | ||
| }; | ||
|
|
||
| export const WORKSPACE_EXPORT_TYPES_DESCENDANT_MAP: Record<string, string[]> = { |
There was a problem hiding this comment.
Ideally, we should declare all the above exported types with the descendant types, but I cannot find some of them, like proto file and profo dir.
Not sure if there are any nesting relations I missed.
There was a problem hiding this comment.
It may be related to grpc but i didnt find them either.
| const queryTypesFromDescendantMap = queryTypesDescendantMap | ||
| ? queryTypesDescendantMap[doc?.type || ''] || [] | ||
| : null; | ||
| const types = queryTypesFromDescendantMap ?? (queryTypes?.length ? queryTypes : allTypes()); |
There was a problem hiding this comment.
It always goes through all types to find out all the valuable items. But in the real world, it's unnecessary. E.g., there is no CookieJar that uses Request as its parent.
|
Comparison with the following code: console.time('[debug] database.withDescendants');
await database.withDescendants(workspace);
console.timeEnd('[debug] database.withDescendants');
console.time('[debug] database.withDescendants with queryTypes');
await database.withDescendants(workspace, null, exportableTypes);
console.timeEnd('[debug] database.withDescendants with queryTypes');
console.time('[debug] get-workspace-from-loader');
await getWorkspaceWithDescendants({ workspaceId });
console.timeEnd('[debug] get-workspace-from-loader');
console.time('[debug] database.withDescendants with descendantMap');
const workspaceDescendants = await database.withDescendants(
workspace,
null,
[],
WORKSPACE_EXPORT_TYPES_DESCENDANT_MAP,
);
console.timeEnd('[debug] database.withDescendants with descendantMap');Almost 100x for a workspace with 100 requests. 16:46:55.044 › [debug] database.withDescendants: 337.384ms
16:46:55.255 › [debug] database.withDescendants with queryTypes: 210.817ms
16:46:55.262 › [debug] get-workspace-from-loader: 6.435ms
16:46:55.265 › [debug] database.withDescendants with descendantMap: 3.51ms
16:46:55.606 › [debug] database.withDescendants: 322.907ms
16:46:55.814 › [debug] database.withDescendants with queryTypes: 207.675ms
16:46:55.821 › [debug] get-workspace-from-loader: 6.911ms
16:46:55.825 › [debug] database.withDescendants with descendantMap: 3.448ms
16:46:56.169 › [debug] database.withDescendants: 326.421ms
16:46:56.373 › [debug] database.withDescendants with queryTypes: 203.712ms
16:46:56.380 › [debug] get-workspace-from-loader: 6.297ms
16:46:56.384 › [debug] database.withDescendants with descendantMap: 4.107ms
16:46:56.418 › [debug] database.withDescendants: 17.164ms
16:46:56.427 › [debug] database.withDescendants with queryTypes: 9.568ms
16:46:56.432 › [debug] get-workspace-from-loader: 4.014ms
16:46:56.434 › [debug] database.withDescendants with descendantMap: 2.051ms |
…earch function to optimize for less database calls
|
|
||
| // If no queryTypes are provided, we want to search all types | ||
| if (queryTypes.length === 0) { | ||
| queryTypes = allTypes(); |
There was a problem hiding this comment.
as queryTypes is an existing arg and probably it should be intersected with the result from queryTypesDescendantMap
| [EXPORT_TYPE_PROTO_DIRECTORY]: protoDirectory, | ||
| }; | ||
|
|
||
| export const WORKSPACE_EXPORT_TYPES_DESCENDANT_MAP: Record<string, string[]> = { |
There was a problem hiding this comment.
It may be related to grpc but i didnt find them either.
|
Hi @gatzjames, I made some changes to withDescendantsV0(develop version) -> withDescendantsV1(first version in this PR) -> withDescendantsV2(the version after your change) -> withDescendantsV3(latest version). It's slower when dealing with a workspace in most cases, but faster when dealing with cases where there are many parent items at the same level. |
Background
INS-694
When there are more than 1000 requests in one collection, some git operations will significantly block the main process, and users can't do anything during that time. This has seriously affected the user experience.
Changes
The performance of
withDescendantsis a bit worse because it always queries all types for each item, so I added aqueryTypesDescendantMapto avoid unnecessary queries.queryTypesDescendantMapindb.withDescendants.WORKSPACE_EXPORT_TYPES_DESCENDANT_MAPto declare the export types.