eclipse-equinox / p2

Eclipse Public License 2.0
14 stars 39 forks source link

Cache loaded repos in ProvisioningContext #517

Open laeubi opened 3 months ago

laeubi commented 3 months ago

Currently if fetching updates the provision context is queried multiple times leading to the repositories loaded multiple times, even though this is usually faster as repo are cached this still produces unnecessary workload, especially if there is one failed repository it will be tried to be loaded over and over again.

This now caches the repositories loaded on first access to speed that up.

github-actions[bot] commented 3 months ago

Test Results

 3 files   -   372   3 suites   - 372   0s :stopwatch: - 42m 50s 26 tests  - 1 867  26 :white_check_mark:  - 1 864  0 :zzz:  - 3  0 :x: ±0  78 runs   - 6 601  78 :white_check_mark:  - 6 592  0 :zzz:  - 9  0 :x: ±0 

Results for commit 85b0b03b. ± Comparison against base commit a287e7e1.

This pull request removes 1867 tests. ``` AutomatedTests org.eclipse.equinox.frameworkadmin.tests.AllTests org.eclipse.equinox.frameworkadmin.tests.Bug196525 ‑ testConfigContent AutomatedTests org.eclipse.equinox.frameworkadmin.tests.AllTests org.eclipse.equinox.frameworkadmin.tests.Bug258370 ‑ testComma AutomatedTests org.eclipse.equinox.frameworkadmin.tests.AllTests org.eclipse.equinox.frameworkadmin.tests.CleanupTest ‑ testOSGiRemoval AutomatedTests org.eclipse.equinox.frameworkadmin.tests.AllTests org.eclipse.equinox.frameworkadmin.tests.CleanupTest ‑ testSimpleConfiguratorRemoval AutomatedTests org.eclipse.equinox.frameworkadmin.tests.AllTests org.eclipse.equinox.frameworkadmin.tests.CleanupTest ‑ testWithMutipleBundles AutomatedTests org.eclipse.equinox.frameworkadmin.tests.AllTests org.eclipse.equinox.frameworkadmin.tests.FrameworkExtensionTest ‑ testAddRemoveFrameworkExtension AutomatedTests org.eclipse.equinox.frameworkadmin.tests.AllTests org.eclipse.equinox.frameworkadmin.tests.LauncherConfigLocationTest ‑ testCustomLauncherConfig AutomatedTests org.eclipse.equinox.frameworkadmin.tests.AllTests org.eclipse.equinox.frameworkadmin.tests.LauncherDataTest ‑ testRemoveProgramArg AutomatedTests org.eclipse.equinox.frameworkadmin.tests.AllTests org.eclipse.equinox.frameworkadmin.tests.ManipulatorTests ‑ testBug212361_osgiInBundlesList AutomatedTests org.eclipse.equinox.frameworkadmin.tests.AllTests org.eclipse.equinox.frameworkadmin.tests.ManipulatorTests ‑ testBug258126_ProgramArgs_VMArgs … ```

:recycle: This comment has been updated with latest results.

merks commented 3 months ago

I guess this same problem doesn't arise for ProvisioningContext.getLoadedArtifactRepositories(IProgressMonitor)?

One concern I have here is that one can call org.eclipse.equinox.p2.engine.ProvisioningContext.setMetadataRepositories(URI...) in the API and result of getLoadedMetadataRepositories depends on that value so probably it would be most proper to clear the cache when that setter is called.

laeubi commented 3 months ago

The ProvisionContext is kind of messy... e.g. it says

The provisioning context has a distinct lifecycle, whereby the metadata and artifact repositories to be used are determined when the client retrieves retrieves the metadata queryable. Clients should not reset the list of metadata repository cations or artifact repository locations once the metadata queryable has been retrieved.

in fact the artifact repositories are only loaded fully when the metadata is queried and so on.

Of course this "distinct lifecycle" is no where described :-\