Share dependency closure computation across classpath updates#2369
Share dependency closure computation across classpath updates#2369vogella wants to merge 2 commits into
Conversation
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
Test Results 126 files ± 0 126 suites ±0 33m 19s ⏱️ - 5m 30s 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. |
Can you please benchmarks comparing the runtimes before and after this change? |
|
@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. |
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.