Skip to content

Share dependency closure computation across classpath updates#2369

Draft
vogella wants to merge 2 commits into
eclipse-pde:masterfrom
vogella:share-dependency-closure
Draft

Share dependency closure computation across classpath updates#2369
vogella wants to merge 2 commits into
eclipse-pde:masterfrom
vogella:share-dependency-closure

Conversation

@vogella

@vogella vogella commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

When the classpath containers of many projects are updated in one run of UpdateClasspathsJob, the transitive dependency closure used for the forbidden-access entries was computed from scratch for every project, although the dependency graphs of the projects in a workspace overlap to a large degree. This change introduces a cache that memoizes the build-relevant closure per bundle and composes a project's closure as the union of the per-bundle closures, so each subgraph is traversed once per job run instead of once per dependent project. The composition is exact because the build-relevant namespaces include Fragment-Host, so a fragment always pulls its host into the same closure. The cache is scoped to a single job run, which makes invalidation unnecessary. Contributes to #2361.

Note: this builds on #2366, the first commit here is that PR and this should be rebased once it is merged.

vogella added 2 commits June 11, 2026 19:58
Replace the work queue of UpdateClasspathsJob with a map keyed by
project so that deduplication happens structurally when a request is
added instead of while draining the queue. This allows UpdateRequest to
become private again and removes the drainRequests helper that was made
public only for testing, together with the isolated unit tests for it.

Addresses review feedback in
eclipse-pde#2363
When the classpath containers of many projects are updated in one run of
UpdateClasspathsJob, the transitive dependency closure used to add
transitive dependencies with forbidden access rules was computed from
scratch for every project, although the dependency graphs of the
projects in a workspace overlap to a large degree.

Introduce DependencyClosureCache, which memoizes the build-relevant
closure per bundle. The closure of a project's direct dependencies is
the union of the per-bundle closures. This composition is exact because
the build-relevant namespaces include Fragment-Host, so a fragment
always pulls its host into the same closure and requirements declared
by attached fragments are followed in every closure containing the
fragment. The cache is scoped to a single job run because the resolved
state can change between runs.

Contributes to eclipse-pde#2361
@github-actions

Copy link
Copy Markdown

Test Results

  126 files  ± 0    126 suites  ±0   33m 19s ⏱️ - 5m 30s
3 511 tests  -  5  3 457 ✅  -  5   54 💤 ±0  0 ❌ ±0 
9 336 runs   - 15  9 206 ✅  - 15  130 💤 ±0  0 ❌ ±0 

Results for commit a0ffbdc. ± Comparison against base commit d0a78c5.

This pull request removes 6 and adds 1 tests. Note that renamed tests count towards both.
org.eclipse.pde.ui.tests.classpathupdater.ClasspathContainerStateTest ‑ deduplicatesRequestsForSameProject
org.eclipse.pde.ui.tests.classpathupdater.ClasspathContainerStateTest ‑ drainsAllRequestsAndPreservesOrder
org.eclipse.pde.ui.tests.classpathupdater.ClasspathContainerStateTest ‑ emptyQueueYieldsNoRequests
org.eclipse.pde.ui.tests.classpathupdater.ClasspathContainerStateTest ‑ latestSavedStateWins
org.eclipse.pde.ui.tests.classpathupdater.ClasspathContainerStateTest ‑ requestWithoutSavedStateWinsOverEarlierSavedState
org.eclipse.pde.ui.tests.classpathupdater.ClasspathContainerStateTest ‑ requestWithoutSavedStateWinsOverLaterSavedState
org.eclipse.pde.core.tests.internal.DependencyManagerTest ‑ testCollectBuildRelevantDependencies_sharedClosureCache

@HannesWell

Copy link
Copy Markdown
Member

This change introduces a cache that memoizes the build-relevant closure per bundle and composes a project's closure as the union of the per-bundle closures, so each subgraph is traversed once per job run instead of once per dependent project.

Can you please benchmarks comparing the runtimes before and after this change?
From my experience that particular code to compute the dependency closure of a bundle is quite fast. So I'm in doubt this helps significantly. Since caching is always difficult and often a source of bugs I'd rather not add it if it doesn't help significantly.
I assume that there are other places or better strategies (like the suggested parallelization) to achieve a greater improvement.

@vogella

vogella commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

@HannesWell I like your thinking that you also like the parallel update better. I have this already locally but assumed that would be even more controversal. :-) I will polish that one and provide it as PR.

My "classpath update is ultra slow so that using Eclipse is basically impossible" is unfortunately on a client side which I'm not allowed to do anything but to look at the code. But fortunately I'm already mitigating to custom IDE builds with custom update sides and may be able to report delta times next week based on this 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

Successfully merging this pull request may close these issues.

2 participants