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

[@nx/eslint] ESLint project initialization is ~6x slower (Nx 17 --> 18) #23004

Closed
1 of 4 tasks
miluoshi opened this issue Apr 25, 2024 · 2 comments
Closed
1 of 4 tasks

Comments

@miluoshi
Copy link
Contributor

Current Behavior

Context

We have a local generator for libs, that uses Nx generators to generate 3 types of libraries: libraryGenerator from @nx/angular/generators, @nx/react and @nx/js.

We are running Jest unit tests on this generator that use Tree created by createTreeWithEmptyWorkspace from @nx/devkit/testing.

Recently we've upgraded Nx from 17.3.2 to 18.3.3 and performance of our tests degraded - now each tests runs 6x slower.

Upgrade to Nx 18

With Nx 17.3.2 each local generator test took 30-40 ms.
After upgrade to 18.0.0 and then to 18.3.3 each test takes 190-250ms.
It doesn't matter which of the 3 types of generators (js, react, angular) we use - all are now slower by approximately same margin.

Finding the root cause

I've started digging into why they are slower, so I've started sprinkling my test code and then source code of Nx library generators with console.time and console.timeEnd statements to find the code that is now slower.

It eventually led me to ESLint project initialization during generation of a lib of any type.

To be specific, this statement added here consumes around 160ms (80%) of whole run time of a library generator:

const graph = await createProjectGraphAsync();

Expected Behavior

Lib generator should not become suddenly 6x slower. And if it is really necessary to have such performance degradation in sake of adding features, it should be possible to avoid doing such expensive computation within unit tests.

GitHub Repo

No response

Steps to Reproduce

  1. Initialize a new Nx workspace on version prior to v18 (e.g. the latest 17.3.2)
  2. Create a Jest test that runs this code:
import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing';
import { libraryGenerator } from '@nx/js'; // or '@nx/react' or '@nx/angular/generators'

test('generates library', async () => {
  const tree = createTreeWithEmptyWorkspace({ layout: 'apps-libs' });

  await libraryGenerator(tree, {
    bundler: 'none',
    directory: 'libs/test-lib', 
    name: 'test-lib',
    projectNameAndRootFormat: 'as-provided',
    tags: [],
    unitTestRunner: 'jest',
  });
});
  1. Run this test multiple times and observe measured test time
  2. Upgrade to Nx >=18.0.0
  3. Run the test again and observe the measured time to be ~6x longer

Below in the Failure logs section I provide logs of the run times of specific methods within the slowest code branch for multiple test runs.

Nx Report

Node   : 20.9.0
OS     : darwin-arm64
yarn   : 1.22.19

nx                 : 18.3.3
@nx/js             : 18.3.3
@nx/jest           : 18.3.3
@nx/linter         : 18.3.3
@nx/eslint         : 18.3.3
@nx/workspace      : 18.3.3
@nx/angular        : 18.3.3
@nx/cypress        : 18.3.3
@nx/devkit         : 18.3.3
@nx/eslint-plugin  : 18.3.3
@nx/plugin         : 18.3.3
@nx/react          : 18.3.3
@nx/rollup         : 18.3.3
@nx/storybook      : 18.3.3
@nrwl/tao          : 18.3.3
@nx/web            : 18.3.3
@nx/webpack        : 18.3.3
typescript         : 5.4.5
---------------------------------------
Community plugins:
@ngrx/component-store    : 17.2.0
@ngrx/effects            : 17.2.0
@ngrx/entity             : 17.2.0
@ngrx/schematics         : 17.2.0
@ngrx/store              : 17.2.0
@storybook/angular       : 7.5.3
@testing-library/angular : 15.0.0
ng2-charts               : 4.1.1
---------------------------------------
Local workspace plugins:
         @<workspace-name>/nx-plugin

Failure Logs

console.time
    @nx/eslint/src/generators/init:initEsLint:devkit.createProjectGraphAsync: 160 ms

      at initEsLint (../../node_modules/@nx/eslint/src/generators/init/init.js:45:13)

  console.time
    @nx/eslint/src/generators/init:initEsLint: 161 ms

      at initEsLint (../../node_modules/@nx/eslint/src/generators/init/init.js:81:13)

  console.time
    addLint:lintProjectGenerator: 163 ms

      at addLint (../../node_modules/@nx/js/src/generators/library/library.js:244:13)

  console.time
    @nx/js/src/generators/library/library.ts:addLint: 163 ms

      at addLint (../../node_modules/@nx/js/src/generators/library/library.js:307:13)

  console.time
    @nx/js/src/generators/library/library.ts:libraryGeneratorInternal: 209 ms

      at libraryGeneratorInternal (../../node_modules/@nx/js/src/generators/library/library.js:135:13)

  console.time
    @nx/js/src/generators/library/library.ts:libraryGenerator: 210 ms

      at timeEnd (src/generators/nx-lib/generator.ts:124:15)

---

  console.time
    @nx/eslint/src/generators/init:initEsLint:devkit.createProjectGraphAsync: 155 ms

      at initEsLint (../../node_modules/@nx/eslint/src/generators/init/init.js:45:13)

  console.time
    @nx/eslint/src/generators/init:initEsLint: 157 ms

      at initEsLint (../../node_modules/@nx/eslint/src/generators/init/init.js:81:13)

  console.time
    addLint:lintProjectGenerator: 159 ms

      at addLint (../../node_modules/@nx/js/src/generators/library/library.js:244:13)

  console.time
    @nx/js/src/generators/library/library.ts:addLint: 159 ms

      at addLint (../../node_modules/@nx/js/src/generators/library/library.js:307:13)

  console.time
    @nx/js/src/generators/library/library.ts:libraryGeneratorInternal: 202 ms

      at libraryGeneratorInternal (../../node_modules/@nx/js/src/generators/library/library.js:135:13)

  console.time
    @nx/js/src/generators/library/library.ts:libraryGenerator: 203 ms

      at timeEnd (src/generators/nx-lib/generator.ts:124:15)

---

  console.time
    @nx/eslint/src/generators/init:initEsLint:devkit.createProjectGraphAsync: 176 ms

      at initEsLint (../../node_modules/@nx/eslint/src/generators/init/init.js:45:13)

  console.time
    @nx/eslint/src/generators/init:initEsLint: 176 ms

      at initEsLint (../../node_modules/@nx/eslint/src/generators/init/init.js:81:13)

  console.time
    addLint:lintProjectGenerator: 178 ms

      at addLint (../../node_modules/@nx/js/src/generators/library/library.js:244:13)

  console.time
    @nx/js/src/generators/library/library.ts:addLint: 180 ms

      at addLint (../../node_modules/@nx/js/src/generators/library/library.js:307:13)

  console.time
    @nx/js/src/generators/library/library.ts:libraryGeneratorInternal: 237 ms

      at libraryGeneratorInternal (../../node_modules/@nx/js/src/generators/library/library.js:135:13)

  console.time
    @nx/js/src/generators/library/library.ts:libraryGenerator: 237 ms

      at timeEnd (src/generators/slido-lib/generator.ts:124:15)

Package Manager Version

Yarn 1.22.19

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

No response

@AgentEnder
Copy link
Member

We've added features in Nx 18 that require generators to read the project graph, which is a bit computationally expensive. You can mock createProjectGraphAsync to avoid this if desired.

@miluoshi
Copy link
Contributor Author

miluoshi commented May 2, 2024

@AgentEnder What features do justify such multifold performance degradation? The commit I've linked above that adds changes responsible for this degradation doesn't have any description (apart from a linked GH issue that just describes desired outcome, but not how it was achieved) while changing thousands lines across hundreds of files. That fact by itself is disturbing to me.

If those added features, whatever they are, are really important for some reason, have you thought implementing them as opt-in instead of using them automatically for all users?

You can mock createProjectGraphAsync to avoid this if desired.

Alternatively, if they are really required, you already provide @nx/devkit/testing utilities. Can't the mocking mechanism be added as a testing utility function, similarly to createTreeWithEmptyWorkspace?

The problem is the test are creating a virtual workspace with createTreeWithEmptyWorkspace, but the createProjectGraphAsync generates graph on top of real file system, which is wrong in context of the test being run and we don't have control over it currently (the function is not being used directly by test).

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants