Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Benchmark nodes-base #9355

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions .github/workflows/benchmark.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
name: Benchmark

on:
pull_request:
paths:
- 'packages/cli/**'
- 'packages/core/**'
- 'packages/workflow/**'
- 'packages/nodes-base/**'
workflow_dispatch:

jobs:
benchmark:
name: Benchmark
runs-on: ubuntu-latest
steps:
- uses: actions/[email protected]

- run: corepack enable

- uses: actions/[email protected]
with:
node-version: 18.x
cache: pnpm

- run: pnpm install --frozen-lockfile

- name: Build
if: ${{ inputs.cacheKey == '' }}
run: pnpm build:backend

- name: Restore cached build artifacts
if: ${{ inputs.cacheKey != '' }}
uses: actions/cache/[email protected]
with:
path: ./packages/**/dist
key: ${{ inputs.cacheKey }}

- name: Build benchmarks
working-directory: packages/nodes-base
run: pnpm benchmark:build

- name: Benchmark nodes-base
uses: CodSpeedHQ/action@v2
with:
working-directory: packages/nodes-base
run: pnpm benchmark:run
10 changes: 7 additions & 3 deletions packages/nodes-base/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
"url": "git+https://github.com/n8n-io/n8n.git"
},
"scripts": {
"benchmark:build": "tsc -p tsconfig.benchmark.json > /dev/null; tsc-alias -p tsconfig.benchmark.json; pnpm build:metadata",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the > /dev/null ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To silence type errors when transpiling test helpers. Did not fix those here to avoid a larger PR.

"benchmark:run": "NODE_ENV=benchmark node dist/test/benchmark.js",
"clean": "rimraf dist .turbo",
"dev": "pnpm watch",
"typecheck": "tsc",
Expand Down Expand Up @@ -808,15 +810,16 @@
]
},
"devDependencies": {
"@codspeed/tinybench-plugin": "^3.1.0",
"@types/amqplib": "^0.10.1",
"@types/aws4": "^1.5.1",
"@types/basic-auth": "^1.1.3",
"@types/cheerio": "^0.22.15",
"@types/cron": "~1.7.1",
"@types/eventsource": "^1.1.2",
"@types/express": "^4.17.21",
"@types/html-to-text": "^9.0.1",
"@types/gm": "^1.25.0",
"@types/html-to-text": "^9.0.1",
"@types/js-nacl": "^1.3.0",
"@types/jsonwebtoken": "^9.0.6",
"@types/lodash": "^4.14.195",
Expand All @@ -835,7 +838,8 @@
"@types/uuid": "^8.3.2",
"@types/xml2js": "^0.4.14",
"eslint-plugin-n8n-nodes-base": "^1.16.0",
"n8n-core": "workspace:*"
"n8n-core": "workspace:*",
"tinybench": "^2.8.0"
},
"dependencies": {
"@kafkajs/confluent-schema-registry": "1.0.6",
Expand All @@ -851,11 +855,11 @@
"csv-parse": "5.5.0",
"currency-codes": "2.1.0",
"eventsource": "2.0.2",
"html-to-text": "9.0.5",
"fast-glob": "3.2.12",
"fflate": "0.7.4",
"get-system-fonts": "2.0.2",
"gm": "1.25.0",
"html-to-text": "9.0.5",
"iconv-lite": "0.6.3",
"ics": "2.40.0",
"isbot": "3.6.13",
Expand Down
26 changes: 26 additions & 0 deletions packages/nodes-base/test/benchmark.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import glob from 'fast-glob';
import Bench from 'tinybench';
import { withCodSpeed } from '@codspeed/tinybench-plugin';
import { setup, workflowToTests as toTests } from './nodes/Helpers';
import { executeWorkflow } from './nodes/ExecuteWorkflow';

async function main() {
const filePaths = await glob('nodes/**/*.workflow.json');

console.log(`Found ${filePaths.length} workflows to benchmark`);

const tests = toTests(filePaths);
const nodeTypes = setup(tests);
const bench = withCodSpeed(new Bench({ time: 0, iterations: 1 })); // @TODO temp config

for (const test of tests) {
bench.add(test.description, async () => {
await executeWorkflow(test, nodeTypes);
});
}

await bench.warmup();
await bench.run();
}

void main();
22 changes: 19 additions & 3 deletions packages/nodes-base/test/nodes/Helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ import { executeWorkflow } from './ExecuteWorkflow';

import { FAKE_CREDENTIALS_DATA } from './FakeCredentialsMap';

const baseDir = path.resolve(__dirname, '../..');
const baseDir = path.resolve(
__dirname,
process.env.NODE_ENV === 'benchmark' ? '../../..' : '../..',
);

const getFakeDecryptedCredentials = (
nodeCredentials: INodeCredentialsDetails,
Expand Down Expand Up @@ -193,6 +196,10 @@ class NodeTypes implements INodeTypes {
return this.nodeTypes[nodeType].type;
}

getKnownTypes(): IDataObject {
return knownNodes;
}

addNode(nodeTypeName: string, nodeType: INodeType | IVersionedNodeType) {
const loadedNode = {
[nodeTypeName]: {
Expand Down Expand Up @@ -255,7 +262,10 @@ export function setup(testData: WorkflowTestData[] | WorkflowTestData) {
level: 'warning',
});
}
const sourcePath = loadInfo.sourcePath.replace(/^dist\//, './').replace(/\.js$/, '.ts');
const sourcePath =
process.env.NODE_ENV === 'benchmark'
? loadInfo.sourcePath
: loadInfo.sourcePath.replace(/^dist\//, './').replace(/\.js$/, '.ts');
const nodeSourcePath = path.join(baseDir, sourcePath);
const credential = new (require(nodeSourcePath)[loadInfo.className])() as ICredentialType;
credentialTypes.addCredential(credentialName, credential);
Expand All @@ -270,7 +280,10 @@ export function setup(testData: WorkflowTestData[] | WorkflowTestData) {
if (!loadInfo) {
throw new ApplicationError(`Unknown node type: ${nodeName}`, { level: 'warning' });
}
const sourcePath = loadInfo.sourcePath.replace(/^dist\//, './').replace(/\.js$/, '.ts');
const sourcePath =
process.env.NODE_ENV === 'benchmark'
? loadInfo.sourcePath
: loadInfo.sourcePath.replace(/^dist\//, './').replace(/\.js$/, '.ts');
const nodeSourcePath = path.join(baseDir, sourcePath);
const node = new (require(nodeSourcePath)[loadInfo.className])() as INodeType;
nodeTypes.addNode(nodeName, node);
Expand Down Expand Up @@ -373,6 +386,9 @@ export const workflowToTests = (workflowFiles: string[]) => {
);
}
});

if (process.env.NODE_ENV === 'benchmark') workflowData.pinData = {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use pinData for assertions. does this change mean that we disable those assertions when running nodes tests for benchmarking?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmarks use executeWorkflow but do not use equalityTest so they sidestep all Jest assertions.

This line specifically I added because a handful of workflows (MoveBinaryData, QuickChart) were getting caught by the pindata check right after this line, because they have their own implementation of testWorkflows.


if (workflowData.pinData === undefined) {
throw new ApplicationError('Workflow data does not contain pinData', { level: 'warning' });
}
Expand Down
9 changes: 9 additions & 0 deletions packages/nodes-base/tsconfig.benchmark.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we do a on-the-fly compilation of TS code (like ts-jest does), instead of needing yet another tsconfig?

"extends": ["./tsconfig.json", "../../tsconfig.build.json"],
"compilerOptions": {
"outDir": "dist/test",
"tsBuildInfoFile": "dist/benchmark.tsbuildinfo"
},
"include": ["test/**/*.ts"],
"exclude": ["test/**/*.test.ts"]
}
Loading
Loading