Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an ArtifactManager to allows unified managing of artifacts #596

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

laeubi
Copy link
Member

@laeubi laeubi commented Jan 5, 2025

Caching can greatly improve performance, basically P2 has two concepts of where caching can occur, the transport and artifacts that can originate from different sources. Currently there is only a CacheManger for handling the first case and nothing for the second, maybe even more important case. Because of this there are different approaches out in the wild e.g. PDE Bundle Pools, Oomph shared P2 pools, and Tycho is using its own cache in the m2 repository, but all of them suffer from the fact that there is no real convenient entry point so different things needs to be enhanced an managed.

This now adds a new abstraction service the ArtifactManager that allows to intercept the process of downloading a logical artifact from a given location and possibly providing an equivalent one that is already present locally. There is also a default implementation that simply delegates to the transport directly as before,

Copy link

github-actions bot commented Jan 5, 2025

Test Results

  375 files  ±0    375 suites  ±0   43m 32s ⏱️ -10s
1 904 tests ±0  1 901 ✅ ±0  3 💤 ±0  0 ❌ ±0 
6 712 runs  ±0  6 703 ✅ ±0  9 💤 ±0  0 ❌ ±0 

Results for commit 4ac7a13. ± Comparison against base commit f295d02.

♻️ This comment has been updated with latest results.

@laeubi
Copy link
Member Author

laeubi commented Jan 5, 2025

I don't get why this fails on windows only :-\

@laeubi laeubi force-pushed the add_artifactmanager2 branch 2 times, most recently from 6330db3 to c3ed58f Compare January 5, 2025 18:44
@mickaelistria
Copy link
Contributor

That seems interesting. Can you please link to some draft PRs (in PDE, or Tycho, or whatever) showcasing how such a new service could be leveraged?
Would it be possible to consider a smarter implementation even directly in p2?

@laeubi
Copy link
Member Author

laeubi commented Jan 6, 2025

Tycho for example has MirroringArtifactProvider where we put a lot of effort and require special setup to have a cache of artifacts what could better be implemented in this way. PDE has similar with its bundle pool where it cache already fetched artifacts.

Would it be possible to consider a smarter implementation even directly in p2?

Everything is possible, currently there is no P2 intrinsic caching mechanism for artifacts. I just found that everyone seems to have slightly different demands on how caching works, so the most challenging would be to define a common cache infrastructure that fits all use-cases.

Caching can greatly improve performance, basically P2 has two concepts
of where caching can occur, the transport and artifacts that can
originate from different sources. Currently there is only a CacheManger
for handling the first case and nothing for the second, maybe even more
important case. Because of this there are different approaches out in
the wild e.g. PDE Bundle Pools, Oomph shared P2 pools, and Tycho is
using its own cache in the m2 repository, but all of them suffer from
the fact that there is no real convenient entry point so different
things needs to be enhanced an managed.

This now adds a new abstraction service the ArtifactManager that allows
to intercept the process of downloading a logical artifact from a given
location and possibly providing an equivalent one that is already
present locally. There is also a default implementation that simply
delegates to the transport directly as before,
@laeubi laeubi force-pushed the add_artifactmanager2 branch from c3ed58f to 4ac7a13 Compare January 6, 2025 09:44
@laeubi laeubi merged commit 7c95f43 into eclipse-equinox:master Jan 6, 2025
12 checks passed
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