Skip to content

Commit

Permalink
Perf: evaluate issue paths lazily
Browse files Browse the repository at this point in the history
refactors 'path' to use 'class LazyPath' instead of
PathItem[].

Issue paths are only needed when flattening errors, so
eagerly constructing them for the ParseInfo objects passed
to every parser results in a lot of unnecessary allocations.

Skipping most of these allocations and evaluating the paths
only when they are needed improves performance.
  • Loading branch information
jussisaurio committed Aug 17, 2023
1 parent 2429f44 commit 0619381
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 37 deletions.
4 changes: 2 additions & 2 deletions library/src/error/ValiError/ValiError.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { PathItem } from '../../types.ts';
import type { LazyPath } from '../../utils/index.ts';

/**
* Issue reason type.
Expand Down Expand Up @@ -36,7 +36,7 @@ export type Issue = {
origin: IssueOrigin;
message: string;
input: any;
path?: PathItem[];
path?: LazyPath;
issues?: Issues;
abortEarly?: boolean;
abortPipeEarly?: boolean;
Expand Down
15 changes: 7 additions & 8 deletions library/src/error/flatten/flatten.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { describe, expect, test } from 'vitest';
import { type Issues, ValiError } from '../ValiError/index.ts';
import { flatten } from './flatten.ts';
import { LazyPath } from '../../utils/index.ts';

describe('flatten', () => {
const rootIssue = {
Expand All @@ -17,14 +18,12 @@ describe('flatten', () => {
origin: 'value' as const,
message: 'Invalid type',
input: { test: 1 },
path: [
{
schema: 'object' as const,
input: { test: 1 },
key: 'test',
value: 1,
},
],
path: new LazyPath(undefined, {
schema: 'object' as const,
input: { test: 1 },
key: 'test',
value: 1,
}),
};

test('should flatten only root error', () => {
Expand Down
2 changes: 1 addition & 1 deletion library/src/error/flatten/flatten.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export function flatten(arg1: ValiError | Issues) {
return (Array.isArray(arg1) ? arg1 : arg1.issues).reduce<FlatErrors>(
(flatErrors, issue) => {
if (issue.path) {
const path = issue.path.map(({ key }) => key).join('.');
const path = issue.path.evaluatedPath.map(({ key }) => key).join('.');
flatErrors.nested[path] = [
...(flatErrors.nested[path] || []),
issue.message,
Expand Down
12 changes: 9 additions & 3 deletions library/src/utils/getPath/getPath.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, test } from 'vitest';
import { getPath } from './getPath.ts';
import { LazyPath, getPath } from './getPath.ts';
import type { PathItem } from '../../types.ts';

describe('getPath', () => {
Expand All @@ -10,7 +10,13 @@ describe('getPath', () => {
key: 'test',
value: 123,
};
expect(getPath(undefined, item)).toEqual([item]);
expect(getPath([item], item)).toEqual([item, item]);
const itemParent = new LazyPath(undefined, {
schema: 'object',
input: { test: 123 },
key: 'test',
value: 123,
});
expect(getPath(undefined, item).evaluatedPath).toEqual([item]);
expect(getPath(itemParent, item).evaluatedPath).toEqual([item, item]);
});
});
49 changes: 36 additions & 13 deletions library/src/utils/getPath/getPath.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,48 @@
import type { PathItem } from '../../types.ts';

/**
* Returns the current path.
* Wrapper for issue paths that only concatenates the full path when it is needed.
*
* @param prevPath Previous, "parent", path. Optional.
* @param pathItem The "leaf" path item to add to the full path.
*
* @returns instance of LazyPath
*/
export class LazyPath {
private prevPath: LazyPath | undefined;
private pathItem: PathItem;
private cachedPath: PathItem[] = [];
constructor(prevPath: LazyPath | undefined, pathItem: PathItem) {
this.prevPath = prevPath;
this.pathItem = pathItem;
}

get evaluatedPath(): PathItem[] {
if (!this.cachedPath.length) {
if (this.prevPath) {
const prevPath = this.prevPath.evaluatedPath;
for (const pathItem of prevPath) {
this.cachedPath.push(pathItem);
}
}
this.cachedPath.push(this.pathItem);
}

return this.cachedPath;
}
}

/**
* Returns a lazy representation of the current path. `path.evaluatedPath` evaluates the path.
*
* TODO: Prüfen ob es wirklich erforderlich ist einen neuen Array zu erstellen.
* TODO: Prüfen ob for loop nur wegen Profiler schneller als spread war
*
* @param info The parse info.
* @param key The current key.
*
* @returns The current path.
* @returns A lazy representation of the current path.
*/
export function getPath(prevPath: PathItem[] | undefined, pathItem: PathItem) {
// Note: Array is copied with for loop instead of spread operator for
// performance reasons
const path: PathItem[] = [];
if (prevPath) {
for (const pathItem of prevPath) {
path.push(pathItem);
}
}
path.push(pathItem);
return path;
export function getPath(prevPath: LazyPath | undefined, pathItem: PathItem) {
return new LazyPath(prevPath, pathItem);
}
15 changes: 7 additions & 8 deletions library/src/utils/getPathInfo/getPathInfo.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import { describe, expect, test } from 'vitest';
import { getPathInfo } from './getPathInfo.ts';
import type { PathItem } from '../../types.ts';
import { LazyPath } from '../getPath/getPath.ts';

describe('getPathInfo', () => {
test('should return pipe info', () => {
const path: PathItem[] = [
{
schema: 'object',
input: { test: null },
key: 'test',
value: null,
},
];
const path = new LazyPath(undefined, {
schema: 'object',
input: { test: null },
key: 'test',
value: null,
});
expect(getPathInfo(undefined, path)).toEqual({
origin: 'value',
path,
Expand Down
5 changes: 3 additions & 2 deletions library/src/utils/getPathInfo/getPathInfo.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { IssueOrigin } from '../../error/index.ts';
import type { ParseInfo, PathItem } from '../../types.ts';
import type { ParseInfo } from '../../types.ts';
import type { LazyPath } from '../getPath/getPath.ts';

/**
* Returns the parse info of a path.
Expand All @@ -11,7 +12,7 @@ import type { ParseInfo, PathItem } from '../../types.ts';
*/
export function getPathInfo(
info: ParseInfo | undefined,
path: PathItem[],
path: LazyPath,
origin: IssueOrigin = 'value'
): ParseInfo {
// Note: The path info is deliberately not constructed with the spread
Expand Down

0 comments on commit 0619381

Please sign in to comment.