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

Feature request: Do not perform unnecessary analysis #978

Open
JavaScriptBach opened this issue Jan 2, 2025 · 3 comments
Open

Feature request: Do not perform unnecessary analysis #978

JavaScriptBach opened this issue Jan 2, 2025 · 3 comments

Comments

@JavaScriptBach
Copy link

Context

I have a Typescript mono-repo with ~30,000 files and would like to write dependency tests of the form "assert there is no dependency path between file A and file B". e.g. a test like this:

{
  from: {
   path: "foo.ts",
  },
  to: {
    path: "bar.ts",
    reachable: true,
  },
}

I'm using depcruise 16.8.0. I've also followed some of the performance advice in #590, e.g. I've limited the module systems and the set of extensions to resolve.

Expected Behavior

depcruise runs quickly and evaluates the rules correctly.

Current Behavior

Depcruise takes 7m28s to run on my mono-repo.

        ∆ rss   ∆ heapTotal    ∆ heapUsed    ∆ external     ⏱  system       ⏱  user       ⏱  real after step...
------------- ------------- ------------- ------------- ------------- ------------- ------------- ------------------------------------------
   +197,956kB     +88,292kB     +56,638kB     +37,720kB         189ms       1,023ms         996ms nodejs starting
     +1,024kB        +256kB        +630kB           0kB           0ms           6ms           5ms parsing options
     -1,796kB      -1,104kB     -13,270kB        +274kB           6ms         114ms          58ms importing analytical modules
     +1,024kB        +512kB     +14,433kB           0kB           9ms         194ms         103ms parsing rule set
          0kB           0kB        +925kB          +1kB           0ms          14ms           9ms determining how to resolve
          0kB           0kB         +17kB           0kB           0ms           0ms           0ms reading files
     +1,920kB        +824kB      +5,609kB        -174kB         132ms         325ms         388ms reading files: gathering initial sources
 +5,154,832kB  +5,106,672kB  +5,015,137kB      +1,575kB       7,386ms      87,148ms      75,799ms reading files: visiting dependencies
          0kB           0kB          +9kB           0kB           0ms           1ms           1ms analyzing
    +43,224kB     +39,784kB     +36,564kB           0kB         235ms     142,665ms     142,528ms analyzing: cycles
     +8,448kB      +8,912kB     +10,116kB           0kB         108ms     224,252ms     224,241ms analyzing: dependents
       +128kB        +208kB      +1,932kB           0kB           0ms          17ms          15ms analyzing: orphans
 -1,891,920kB  -1,896,468kB  -4,419,218kB      -1,679kB         341ms       2,885ms       2,040ms analyzing: reachables
          0kB           0kB         +10kB           0kB           0ms           0ms           0ms analyzing: module metrics
          0kB           0kB          +4kB           0kB           0ms           0ms           0ms analyzing: add focus (if any)
       +384kB        +416kB    +102,658kB           0kB           4ms         519ms         360ms analyzing: validations
    +12,464kB     +12,496kB    +158,404kB           0kB          17ms       1,008ms         822ms reporting

Possible Solution

Can we avoid performing unnecessary analysis if the user did not specify any rules that require them? In my use case, I would not expect depcruise to have to analyze "cycles" or "dependents" in order to tell me if a path exists from file A to file B, both of which I provide. This would reduce the runtime by ~80%.

Considered alternatives

N/A

@sverweij
Copy link
Owner

sverweij commented Jan 4, 2025

Hi @JavaScriptBach yup - we had this at one point in the history of dependency-cruiser, but decided to remove it for simplicity's sake. Most instance of dependency-cruiser would have configuration that'd include cycles and/ or dependents anyway, and the overhead was manageable in the monorepos I ran this on (granted, 'only' 17k files/ 65k dependencies, but cycle detection takes 2.4s there (6% of the total run time)).

That said, it is possible to bring this back. It'll need some thought to ensure this is won't be a breaking change.

Let's save some time (and trees).

@sverweij
Copy link
Owner

sverweij commented Jan 5, 2025

Hi again @JavaScriptBach - I've just published [email protected] to npmjs. It implements an option that, when switched to true, skips cycle, dependents and orphans analysis when rules don't ask for them (the other analysis steps already did so out of the box).

Add skipAnalysisNotInRules: true to the options section of your .dependency-cruiser.js and Bob should be your uncle. Let me know if it works for you (and also, especially if it doesn't 😄 !).

@JavaScriptBach
Copy link
Author

@sverweij Thanks for the prompt implementation! It's working fine on my simple example and has reduced the runtime from 7m28s to 1m23s.

sverweij added a commit that referenced this issue Jan 7, 2025
## Description

- adds a `skipAnalysisNotInRules` option that, when switched to `true`
skips all analyses not necessary for checking the current rule set.
- Defaults the option to `false` for backwards compatibility of both cli
and api.
- Add this option with the value `true` to the --init template that
scaffolds initial .dependency-cruiser.js configurations
- Takes the new option into account in the cache-dirty check
- Adds a paragraph in the options reference

TODO:
- [x] implement for cycle analysis
- [x] implement for dependents analysis
- [x] implement for orphan check

For the _focus_, _metrics_ and _reachables_ analyses this was already in
place by default

## Motivation and Context

Addresses #978 

## How Has This Been Tested?

- [x] green ci
- [x] additional and updated automated non-regression tests

## Types of changes

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Documentation only change
- [ ] Refactor (non-breaking change which fixes an issue without
changing functionality)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants