Skip to content

Commit

Permalink
Simplify ProgressMonitor usage in ProvisioningContext
Browse files Browse the repository at this point in the history 
- Use SubMonitor.split() over newChild()
- Remove explicit checks for cancellation since SubMonitor.split() does
that implicitly
- Simplify work computation by avoiding unnecessary multiplies
	When creating a split SubMonitor the splitted child only operates on
the assigned amount of work.
  • Loading branch information
@HannesWell
HannesWell committed Sep 11, 2023
1 parent 5bf07ad commit 191e622
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 61 deletions.
2 changes: 1 addition & 1 deletion bundles/org.eclipse.equinox.p2.engine/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.equinox.p2.engine;singleton:=true
Bundle-Version: 2.8.100.qualifier
Bundle-Version: 2.8.200.qualifier
Bundle-Activator: org.eclipse.equinox.internal.p2.engine.EngineActivator
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Expand Down
103 changes: 43 additions & 60 deletions .../org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/p2/engine/ProvisioningContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
public class ProvisioningContext {
private IProvisioningAgent agent;
private URI[] artifactRepositories; //artifact repositories to consult
private final List<IInstallableUnit> extraIUs = Collections.synchronizedList(new ArrayList<IInstallableUnit>());
private final List<IInstallableUnit> extraIUs = Collections.synchronizedList(new ArrayList<>());
private URI[] metadataRepositories; //metadata repositories to consult
private final Map<String, String> properties = new HashMap<>();
private Map<String, URI> referencedArtifactRepositories = null;
Expand Down Expand Up @@ -171,35 +171,33 @@ private List<IArtifactRepository> getLoadedArtifactRepositories(IProgressMonitor
Arrays.sort(repositories, LOCAL_FIRST_COMPARATOR);

List<IArtifactRepository> repos = new ArrayList<>();
SubMonitor sub = SubMonitor.convert(monitor, (repositories.length + 1) * 100);
SubMonitor sub = SubMonitor.convert(monitor, repositories.length + 1);
for (URI location : repositories) {
getLoadedRepository(location, repoManager, repos, sub);
getLoadedRepository(location, repoManager, repos, sub.split(1));
// Remove this URI from the list of extra references if it is there.
if (referencedArtifactRepositories != null && location != null) {
referencedArtifactRepositories.remove(location.toString());
}
}
// Are there any extra artifact repository references to consider?
if (referencedArtifactRepositories != null && referencedArtifactRepositories.size() > 0 && shouldFollowArtifactReferences()) {
SubMonitor innerSub = SubMonitor.convert(sub.newChild(100), referencedArtifactRepositories.size() * 100);
SubMonitor innerSub = SubMonitor.convert(sub.split(1), referencedArtifactRepositories.size());
for (URI referencedURI : referencedArtifactRepositories.values()) {
getLoadedRepository(referencedURI, repoManager, repos, innerSub);
getLoadedRepository(referencedURI, repoManager, repos, innerSub.split(1));
}
}
return repos;
}

private void getLoadedRepository(URI location, IArtifactRepositoryManager repoManager, List<IArtifactRepository> repos, SubMonitor monitor) {
if (monitor.isCanceled()) {
throw new OperationCanceledException();
}
private void getLoadedRepository(URI location, IArtifactRepositoryManager repoManager,
List<IArtifactRepository> repos, IProgressMonitor monitor) {
if (failedArtifactRepositories.contains(location)) {
return;
}
try {
IArtifactRepository repository = loadedArtifactRepositories.get(location);
if (repository == null) {
repository = repoManager.loadRepository(location, monitor.newChild(100));
repository = repoManager.loadRepository(location, monitor);
loadedArtifactRepositories.put(location, repository);
}
repos.add(repository);
Expand All @@ -213,37 +211,33 @@ private Set<IMetadataRepository> getLoadedMetadataRepositories(IProgressMonitor
IMetadataRepositoryManager repoManager = agent.getService(IMetadataRepositoryManager.class);
URI[] repositories = metadataRepositories == null ? repoManager.getKnownRepositories(IRepositoryManager.REPOSITORIES_ALL) : metadataRepositories;

HashMap<String, IMetadataRepository> repos = new HashMap<>();
SubMonitor sub = SubMonitor.convert(monitor, repositories.length * 100);
Map<String, IMetadataRepository> repos = new HashMap<>();
SubMonitor sub = SubMonitor.convert(monitor, repositories.length);

// Clear out the list of remembered artifact repositories
referencedArtifactRepositories = new HashMap<>();
for (URI repositorie : repositories) {
if (sub.isCanceled())
throw new OperationCanceledException();
loadMetadataRepository(repoManager, repositorie, repos, shouldFollowReferences(), sub.newChild(100));
loadMetadataRepository(repoManager, repositorie, repos, shouldFollowReferences(), sub.split(1));
}
Set<IMetadataRepository> set = new HashSet<>();
set.addAll(repos.values());
return set;
}

private void loadMetadataRepository(IMetadataRepositoryManager manager, URI location, HashMap<String, IMetadataRepository> repos, boolean followMetadataRepoReferences, IProgressMonitor monitor) {
if (monitor.isCanceled()) {
throw new OperationCanceledException();
}
private void loadMetadataRepository(IMetadataRepositoryManager manager, URI location,
Map<String, IMetadataRepository> repos, boolean followMetadataRepoReferences, IProgressMonitor monitor) {
// if we've already processed this repo, don't do it again. This keeps us from getting
// caught up in circular references.
if (repos.containsKey(location.toString()) || failedMetadataRepositories.contains(location)) {
return;
}

SubMonitor sub = SubMonitor.convert(monitor, 1000);
SubMonitor subMonitor = SubMonitor.convert(monitor, 2);
// First load the repository itself.
IMetadataRepository repository = loadedMetadataRepositories.get(location);
if (repository == null) {
try {
repository = manager.loadRepository(location, sub.newChild(500));
repository = manager.loadRepository(location, subMonitor.split(1));
loadedMetadataRepositories.put(location, repository);
} catch (ProvisionException e) {
failedMetadataRepositories.add(location);
Expand All @@ -254,25 +248,22 @@ private void loadMetadataRepository(IMetadataRepositoryManager manager, URI loca
Collection<IRepositoryReference> references = repository.getReferences();
// We always load artifact repositories referenced by this repository. We might load
// metadata repositories
if (references.size() > 0) {
if (!references.isEmpty()) {
IArtifactRepositoryManager artifactManager = agent.getService(IArtifactRepositoryManager.class);
SubMonitor repoSubMon = SubMonitor.convert(sub.newChild(500), 100 * references.size());
SubMonitor repoMon = SubMonitor.convert(subMonitor.split(1), references.size());
for (IRepositoryReference ref : references) {
try {
if (ref.getType() == IRepository.TYPE_METADATA && followMetadataRepoReferences && isEnabled(manager, ref)) {
loadMetadataRepository(manager, ref.getLocation(), repos, followMetadataRepoReferences, repoSubMon.newChild(100));
} else if (ref.getType() == IRepository.TYPE_ARTIFACT) {
loadMetadataRepository(manager, ref.getLocation(), repos, followMetadataRepoReferences, repoMon.split(1));
} else if (ref.getType() == IRepository.TYPE_ARTIFACT && isEnabled(artifactManager, ref)) {
// We want to remember all enabled artifact repository locations.
if (isEnabled(artifactManager, ref))
referencedArtifactRepositories.put(ref.getLocation().toString(), ref.getLocation());
referencedArtifactRepositories.put(ref.getLocation().toString(), ref.getLocation());
}
} catch (IllegalArgumentException e) {
// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=311338
// ignore invalid location and keep going
}
}
} else {
sub.done();
}

}
Expand Down Expand Up @@ -356,54 +347,50 @@ private interface Manager<T> {

private Collection<IMetadataRepository> getAllLoadedMetadataRepositories(IProgressMonitor monitor) {
if (allLoadedMetadataRepositories == null) {
SubMonitor subMonitor = SubMonitor.convert(monitor, 2);
var repoManager = agent.getService(IMetadataRepositoryManager.class);
getLoadedMetadataRepositories(monitor);
getLoadedMetadataRepositories(subMonitor.split(1));
allLoadedMetadataRepositories = getAllLoadedRepositories(repoManager::loadRepository,
loadedMetadataRepositories, failedMetadataRepositories, monitor);
loadedMetadataRepositories, failedMetadataRepositories, subMonitor.split(1));
}
return allLoadedMetadataRepositories.values();
}

private Collection<IArtifactRepository> getAllLoadedArtifactRepositories(IProgressMonitor monitor) {
if (allLoadedArtifactRepositories == null) {
SubMonitor subMonitor = SubMonitor.convert(monitor, 2);
var repoManager = agent.getService(IArtifactRepositoryManager.class);
getLoadedArtifactRepositories(monitor);
getLoadedArtifactRepositories(subMonitor.split(1));
allLoadedArtifactRepositories = getAllLoadedRepositories(repoManager::loadRepository,
loadedArtifactRepositories, failedArtifactRepositories, monitor);
loadedArtifactRepositories, failedArtifactRepositories, subMonitor.split(1));
}
return allLoadedArtifactRepositories.values();
}

private <T> Map<URI, T> getAllLoadedRepositories(Manager<T> manager,
Map<URI, T> loadedRepositories,
Set<URI> failedRepositories,
IProgressMonitor monitor) {
private <T> Map<URI, T> getAllLoadedRepositories(Manager<T> manager, Map<URI, T> loadedRepositories,
Set<URI> failedRepositories, IProgressMonitor monitor) {
SubMonitor subMonitor = SubMonitor.convert(monitor, loadedRepositories.size());
var allLoadedRepositories = new HashMap<>(loadedRepositories);
if (!loadedRepositories.isEmpty()) {
for (var repository : loadedRepositories.values()) {
loadComposites(manager, repository, allLoadedRepositories, failedRepositories, monitor);
if (monitor.isCanceled()) {
throw new OperationCanceledException();
}
}
for (var repository : loadedRepositories.values()) {
loadComposites(manager, repository, allLoadedRepositories, failedRepositories, subMonitor.split(1));
}
return allLoadedRepositories;
}

private <T> void loadComposites(Manager<T> manager, T repository, Map<URI, T> repos, Set<URI> failedRepositories,
IProgressMonitor monitor) {
if (repository instanceof ICompositeRepository<?> composite) {
for (var location : composite.getChildren()) {
loadRepository(manager, location, repos, failedRepositories, monitor);
List<URI> children = composite.getChildren();
SubMonitor subMonitor = SubMonitor.convert(monitor, children.size());
for (var location : children) {
loadRepository(manager, location, repos, failedRepositories, subMonitor.split(1));
}
}
}

private <T> void loadRepository(Manager<T> manager, URI location, Map<URI, T> repos,
Set<URI> failedRepositories, IProgressMonitor monitor) {
if (monitor.isCanceled()) {
throw new OperationCanceledException();
}
private <T> void loadRepository(Manager<T> manager, URI location, Map<URI, T> repos, Set<URI> failedRepositories,
IProgressMonitor monitor) {
SubMonitor subMonitor = SubMonitor.convert(monitor, 2);

if (repos.containsKey(location) || failedMetadataRepositories.contains(location)) {
return;
Expand All @@ -412,22 +399,18 @@ private <T> void loadRepository(Manager<T> manager, URI location, Map<URI, T> re
var repository = repos.get(location);
if (repository == null) {
try {
repository = manager.loadRepository(location, monitor);
repository = manager.loadRepository(location, subMonitor.split(1));
repos.put(location, repository);
loadComposites(manager, repository, repos, failedRepositories, monitor);
loadComposites(manager, repository, repos, failedRepositories, subMonitor.split(1));
} catch (ProvisionException e) {
failedMetadataRepositories.add(location);
return;
}
}
}

private static Comparator<IArtifactKey> ARTIFACT_KEY_COMPARATOR = (o1, o2) -> {
var cmp = o1.getId().compareTo(o2.getId());
if (cmp == 0)
cmp = o1.getVersion().compareTo(o2.getVersion());
return cmp;
};
private static final Comparator<IArtifactKey> ARTIFACT_KEY_COMPARATOR = Comparator //
.comparing(IArtifactKey::getId) //
.thenComparing(IArtifactKey::getVersion);

/**
* Returns a map from simple artifact repository location to a subset of the
Expand Down

0 comments on commit 191e622

Please sign in to comment.