Skip to content

Find all elementary dependency cycles using Johnson's algorithm#65

Merged
vogella merged 2 commits into
mainfrom
detect-all-dependency-cycles
Jun 12, 2026
Merged

Find all elementary dependency cycles using Johnson's algorithm#65
vogella merged 2 commits into
mainfrom
detect-all-dependency-cycles

Conversation

@vogella

@vogella vogella commented Jun 11, 2026

Copy link
Copy Markdown
Member

The cyclic dependency detector in com.vogella.ide.debugtools used a DFS with parent pointers, which only reports cycles reachable along its particular DFS tree and silently misses overlapping cycles in dense dependency graphs. This PR replaces it with Johnson's algorithm, so every elementary cycle is found exactly once, rooted at its alphabetically smallest plug-in. This also made the old cycle normalization and deduplication code unnecessary, and a cap of 1000 cycles (reported in the console when hit) guards against exponential blow-up.

The PR also removes an unused jakarta.annotation package import from the debugtools manifest. It failed to resolve against the 2026-06 target platform and broke the build of this bundle, verified fixed with mvnw verify -pl com.vogella.ide.debugtools.

The previous DFS with parent pointers only reported cycles reachable
along its particular DFS tree, so overlapping cycles in dense graphs
were missed. The detector now enumerates every elementary cycle exactly
once via Johnson's algorithm (per-vertex strongly connected component
plus blocked-set circuit search), which also makes the old cycle
normalization and deduplication unnecessary. A safety cap of 1000
cycles guards against exponential blow-up and is reported in the
console when hit.

Also removes the unused jakarta.annotation import from the debugtools
manifest, which failed to resolve against the 2026-06 target platform
and broke the build of this bundle.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the cyclic dependency detection in DetectCyclicDependenciesHandler to use Johnson's algorithm for enumerating all elementary cycles exactly once, replacing the previous DFS-based cycle detector. It also removes the unused jakarta.annotation import from the manifest and adds a limit of 1000 cycles to prevent excessive output. The review feedback points out a bug where self-cycles could be recorded twice if the plug-in is also part of a larger strongly connected component, and suggests only recording the cycle in circuit() if the path length is greater than 1.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +345 to +347
if (next.equals(start)) {
recordCycle();
foundCycle = true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

If a plug-in has a self-cycle (i.e., a dependency on itself) and is also part of a larger strongly connected component (size > 1), the self-cycle will be detected and recorded twice: once in findAllCycles() (lines 290-294) and once in circuit() when node is start and next is start (since next.equals(start) is true and path has size 1).

To prevent this duplicate detection, we should only record the cycle in circuit() if the path length is greater than 1 (i.e., path.size() > 1).

Suggested change
if (next.equals(start)) {
recordCycle();
foundCycle = true;
if (next.equals(start)) {
if (path.size() > 1) {
recordCycle();
foundCycle = true;
}
}

A plug-in with a dependency on itself that is also part of a larger
strongly connected component was reported both by the self-loop check
in findAllCycles() and by the trivial path in circuit(). Skip the
trivial path so each self-cycle is only recorded once.
@vogella
vogella merged commit 492504e into main Jun 12, 2026
1 check failed
@vogella
vogella deleted the detect-all-dependency-cycles branch June 12, 2026 17:35
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

Successfully merging this pull request may close these issues.

1 participant