Share dependency closure computation across classpath updates#2369
Share dependency closure computation across classpath updates#2369vogella wants to merge 2 commits into
Conversation
Test Results 129 files ± 0 129 suites ±0 38m 46s ⏱️ +51s Results for commit a21d418. ± Comparison against base commit dd790b2. This pull request removes 6 and adds 1 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
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. |
vogella
force-pushed
the
share-dependency-closure
branch
from
a0ffbdc to
fc54de3
Compare
|
Today I have the opportunity to work again for the client with the slow PDE dependencies update. Wall time for updating the classpath entries WITH the -Dpde.addTransitiveDependenciesWithForbiddenAccess=false flag. Without it I stopped the process last week after 30 mins at around 27% percentage, I do not have time to test that again. (Press Reload in the target platform and watch then the progress monitor says: Updating plug-in dependencies until this entry is replaced with compiling). Range: ~ 4min to almost 7 min I know the range is huge but it is a v-machine, so I assume it is highly affected by the usage of other users. |
|
With #2374 I could measure "correctly". |
Do you mean with just this change applied the runtime dropped from much over 30min to ~ 4min to almost 7 min? |
No, this was the range with -Dpde.addTransitiveDependenciesWithForbiddenAccess=false and no patches applied. 30 min was the time without the above parameter, and I canceled the update before it finished. The number for this patch and the parallel together are in #2375. I did not have the luxary to test this patch in isolation, the client pressed me to work on a production issue. I only patched my Eclipse as otherwise the work as almost impossible to do. |
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
vogella
force-pushed
the
share-dependency-closure
branch
from
fc54de3 to
a21d418
Compare
2 participants
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.