Skip to content

Commit

Permalink
Expose the permutation data through the felix resolver logger
Browse files Browse the repository at this point in the history
Currently it is not really possible to write a test that asserts
something about the complexity of a problem that the resolver has
performed. This has the risk of undetected performance regressions but
also means performance improvements can not be easily investigated. Also
in general code (like for example console commands) it could be useful
to gather some statistics.

This now adds two new methods to the logger, one that is performing a
log each time when a permutation is added or processed to either log
this by the caller or perform any statistics on them.
  • Loading branch information
laeubi committed Apr 30, 2024
1 parent 558580b commit d5e0f49
Show file tree
Hide file tree
Showing 13 changed files with 224 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1981,20 +1981,20 @@ public void testUses4() throws BundleException, IOException {
*/
@Test
public void testUses5Importer() throws BundleException, IOException {
doTestUses5("uses.k.importer.MF");
doTestUses5("uses.k.importer.MF", 3);
}

@Test
public void testUses5ReqCap() throws BundleException, IOException {
doTestUses5("uses.k.reqCap.MF");
doTestUses5("uses.k.reqCap.MF", 3);
}

@Test
public void testUses5Requirer() throws BundleException, IOException {
doTestUses5("uses.k.requirer.MF");
doTestUses5("uses.k.requirer.MF", 3);
}

public void doTestUses5(String kManifest) throws BundleException, IOException {
public void doTestUses5(String kManifest, int max) throws BundleException, IOException {
DummyContainerAdaptor adaptor = createDummyAdaptor();
ModuleContainer container = adaptor.getContainer();

Expand All @@ -2006,12 +2006,13 @@ public void doTestUses5(String kManifest) throws BundleException, IOException {
Module uses_m_conflict1 = installDummyModule("uses.m.conflict1.MF", "m.conflict1", container);
Module uses_m_conflict2 = installDummyModule("uses.m.conflict2.MF", "m.conflict2", container);

container.resolve(null, false);
ResolutionReport report = container.resolve(null, false);

assertEquals("k should resolve.", State.RESOLVED, uses_k.getState());
assertEquals("l should resolve.", State.RESOLVED, uses_l.getState());
assertEquals("m.conflict1 should resolve.", State.RESOLVED, uses_m_conflict1.getState());
assertEquals("m.conflict2 should resolve.", State.RESOLVED, uses_m_conflict2.getState());
assertNotMoreThanPermutationCreated(report, max);
}

@Test
Expand Down Expand Up @@ -3929,14 +3930,28 @@ public void testSubstitutionWithMoreThan2Providers() throws BundleException, IOE
"osgi.ee; osgi.ee=JavaSE; version:List<Version>=\"1.3, 1.4, 1.5, 1.6, 1.7\"", //
container);
ResolutionReport report = container.resolve(Arrays.asList(systemBundle), true);
assertNull("Failed to resolve test.", report.getResolutionException());
assertSucessfulWith(report, 1);

List<Module> modules = new ArrayList<>();
for (String manifest : HTTPCOMPS_AND_EATHER) {
modules.add(installDummyModule(manifest, manifest, container));
}
report = container.resolve(modules, true);
assertSucessfulWith(report, 115);
}

protected void assertSucessfulWith(ResolutionReport report, int maxTotalPermutations) {
assertNull("Failed to resolve test.", report.getResolutionException());
assertNotMoreThanPermutationCreated(report, maxTotalPermutations);
}

protected void assertNotMoreThanPermutationCreated(ResolutionReport report, int maxTotal) {
int totalPermutations = report.getTotalPermutations();
if (totalPermutations > maxTotal) {
fail("Maximum of " + maxTotal + " permutations expected but was " + totalPermutations + " ("
+ report.getProcessedPermutations() + " processed).");
}
return;
}

@Test
Expand Down
11 changes: 11 additions & 0 deletions bundles/org.eclipse.osgi/.settings/.api_filters
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<component id="org.eclipse.osgi" version="2">
<resource path="supplement/src/org/eclipse/osgi/report/resolution/ResolutionReport.java" type="org.eclipse.osgi.report.resolution.ResolutionReport">
<filter id="403853384">
<message_arguments>
<message_argument value="@noimplement"/>
<message_argument value="org.eclipse.osgi.report.resolution.ResolutionReport"/>
</message_arguments>
</filter>
</resource>
</component>
4 changes: 2 additions & 2 deletions bundles/org.eclipse.osgi/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ Export-Package: org.eclipse.core.runtime.adaptor;x-friends:="org.eclipse.core.ru
org.eclipse.osgi.internal.signedcontent;x-internal:=true,
org.eclipse.osgi.internal.url;x-internal:=true,
org.eclipse.osgi.launch;version="1.1";uses:="org.osgi.framework,org.osgi.framework.launch,org.osgi.framework.connect",
org.eclipse.osgi.report.resolution;version="1.0";uses:="org.osgi.service.resolver,org.osgi.resource",
org.eclipse.osgi.report.resolution;version="1.1.0";uses:="org.osgi.service.resolver,org.osgi.resource",
org.eclipse.osgi.service.datalocation;version="1.4.0",
org.eclipse.osgi.service.debug;version="1.2",
org.eclipse.osgi.service.environment;version="1.4",
Expand Down Expand Up @@ -107,7 +107,7 @@ Bundle-Activator: org.eclipse.osgi.internal.framework.SystemBundleActivator
Bundle-Description: %systemBundle
Bundle-Copyright: %copyright
Bundle-Vendor: %eclipse.org
Bundle-Version: 3.19.100.qualifier
Bundle-Version: 3.20.0.qualifier
Bundle-Localization: systembundle
Bundle-DocUrl: http://www.eclipse.org
Eclipse-ExtensibleAPI: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

import org.eclipse.osgi.container.Module.StartOptions;
import org.eclipse.osgi.container.Module.State;
import org.eclipse.osgi.container.Module.StopOptions;
Expand Down Expand Up @@ -651,7 +650,8 @@ public ResolutionReport resolve(Collection<Module> triggers, boolean triggersMan
private ResolutionReport resolve(Collection<Module> triggers, boolean triggersMandatory, boolean restartTriggers) {
if (isRefreshingSystemModule()) {
return new ModuleResolutionReport(null, Collections.emptyMap(),
new ResolutionException("Unable to resolve while shutting down the framework.")); //$NON-NLS-1$
new ResolutionException("Unable to resolve while shutting down the framework."), -1, -1, -1, -1, //$NON-NLS-1$
-1);
}
ResolutionReport report = null;
try (ResolutionLock.Permits resolutionPermits = _resolutionLock.acquire(1)) {
Expand All @@ -664,15 +664,16 @@ private ResolutionReport resolve(Collection<Module> triggers, boolean triggersMa
if (be.getType() == BundleException.REJECTED_BY_HOOK
|| be.getType() == BundleException.STATECHANGE_ERROR) {
return new ModuleResolutionReport(null, Collections.emptyMap(),
new ResolutionException(be));
new ResolutionException(be), -1, -1, -1, -1, -1);
}
}
throw e;
}
} while (report == null);
} catch (ResolutionLockException e) {
return new ModuleResolutionReport(null, Collections.emptyMap(),
new ResolutionException("Timeout acquiring lock for resolution", e, Collections.emptyList())); //$NON-NLS-1$
new ResolutionException("Timeout acquiring lock for resolution", e, Collections.emptyList()), -1, //$NON-NLS-1$
-1, -1, -1, -1);
}
return report;
}
Expand Down Expand Up @@ -1341,7 +1342,7 @@ public ResolutionReport refresh(Collection<Module> initial) {
if (!isRefreshingSystemModule()) {
return resolve(refreshTriggers, false, true);
}
return new ModuleResolutionReport(null, null, null);
return new ModuleResolutionReport(null, null, null, -1, -1, -1, -1, -1);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.eclipse.osgi.container.ModuleResolver.ResolveProcess.ResolveLogger;
import org.eclipse.osgi.internal.messages.Msg;
import org.eclipse.osgi.report.resolution.ResolutionReport;
import org.osgi.framework.wiring.BundleRevision;
Expand Down Expand Up @@ -49,9 +50,12 @@ public void addEntry(Resource resource, Entry.Type type, Object data) {
entries.add(new EntryImpl(type, data));
}

public ModuleResolutionReport build(Map<Resource, List<Wire>> resolutionResult, ResolutionException cause) {
return new ModuleResolutionReport(resolutionResult, resourceToEntries, cause);
public ModuleResolutionReport build(Map<Resource, List<Wire>> resolutionResult, ResolutionException cause,
ResolveLogger logger) {
return new ModuleResolutionReport(resolutionResult, resourceToEntries, cause, logger.totalPerm,
logger.processedPerm, logger.usesPerm, logger.subPerm, logger.importPerm);
}

}

static class EntryImpl implements Entry {
Expand All @@ -77,9 +81,19 @@ public Type getType() {
private final Map<Resource, List<Entry>> entries;
private final ResolutionException resolutionException;
private final Map<Resource, List<Wire>> resolutionResult;
private int totalPerm;
private int processedPerm;
private int usesPerm;
private int subPerm;
private int importPerm;

ModuleResolutionReport(Map<Resource, List<Wire>> resolutionResult, Map<Resource, List<Entry>> entries,
ResolutionException cause) {
ResolutionException cause, int totalPerm, int processedPerm, int usesPerm, int subPerm, int importPerm) {
this.totalPerm = totalPerm;
this.processedPerm = processedPerm;
this.usesPerm = usesPerm;
this.subPerm = subPerm;
this.importPerm = importPerm;
this.entries = entries == null ? Collections.emptyMap() : Collections.unmodifiableMap(new HashMap<>(entries));
this.resolutionResult = resolutionResult == null ? Collections.emptyMap()
: Collections.unmodifiableMap(resolutionResult);
Expand Down Expand Up @@ -176,4 +190,29 @@ private static void printResolutionEntry(StringBuilder result, String prepend, R
public String getResolutionReportMessage(Resource resource) {
return getResolutionReport0(null, (ModuleRevision) resource, getEntries(), null);
}

@Override
public int getTotalPermutations() {
return totalPerm;
}

@Override
public int getProcessedPermutations() {
return processedPerm;
}

@Override
public int getUsesPermutations() {
return usesPerm;
}

@Override
public int getImportPermutations() {
return importPerm;
}

@Override
public int getSubstitutionPermutations() {
return subPerm;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.felix.resolver.Logger;
import org.apache.felix.resolver.PermutationType;
import org.apache.felix.resolver.ResolutionError;
import org.apache.felix.resolver.ResolverImpl;
import org.eclipse.osgi.container.ModuleRequirement.DynamicModuleRequirement;
Expand Down Expand Up @@ -89,11 +90,13 @@ final class ModuleResolver {
private static final String OPTION_USES = OPTION_RESOLVER + "/uses"; //$NON-NLS-1$
private static final String OPTION_WIRING = OPTION_RESOLVER + "/wiring"; //$NON-NLS-1$
private static final String OPTION_REPORT = OPTION_RESOLVER + "/report"; //$NON-NLS-1$
private static final String OPTION_PERMUTATION = OPTION_RESOLVER + "/permutation"; //$NON-NLS-1$

boolean DEBUG_ROOTS = false;
boolean DEBUG_PROVIDERS = false;
boolean DEBUG_HOOKS = false;
boolean DEBUG_USES = false;
boolean DEBUG_PERMUTATIONS = false;
boolean DEBUG_WIRING = false;
boolean DEBUG_REPORT = false;

Expand All @@ -115,6 +118,7 @@ void setDebugOptions() {
DEBUG_USES = debugAll || options.getBooleanOption(OPTION_USES, false);
DEBUG_WIRING = debugAll || options.getBooleanOption(OPTION_WIRING, false);
DEBUG_REPORT = debugAll || options.getBooleanOption(OPTION_REPORT, false);
DEBUG_PERMUTATIONS = debugAll || options.getBooleanOption(OPTION_PERMUTATION, false);
}

static final Collection<String> NON_PAYLOAD_CAPABILITIES = Arrays.asList(IdentityNamespace.IDENTITY_NAMESPACE);
Expand Down Expand Up @@ -470,6 +474,11 @@ class ResolveProcess extends ResolveContext implements Comparator<Capability>, E

class ResolveLogger extends Logger {
private Map<Resource, ResolutionException> errors = null;
public int totalPerm;
public int processedPerm;
public int usesPerm;
public int importPerm;
public int subPerm;

public ResolveLogger() {
super(DEBUG_USES ? Logger.LOG_DEBUG : 0);
Expand Down Expand Up @@ -510,6 +519,28 @@ protected void doLog(int level, String msg, Throwable throwable) {
+ (throwable != null ? (TAB + TAB + throwable.getMessage()) : "")); //$NON-NLS-1$
}

@Override
public void logPermutationAdded(PermutationType type) {
totalPerm++;
switch (type) {
case USES:
usesPerm++;
break;
case IMPORT:
importPerm++;
break;
case SUBSTITUTE:
subPerm++;
}
}

PermutationType[] permutationTypes = PermutationType.values();

@Override
public void logProcessPermutation(PermutationType type) {
processedPerm++;
}

}

private final ModuleResolutionReport.Builder reportBuilder = new ModuleResolutionReport.Builder();
Expand Down Expand Up @@ -918,7 +949,7 @@ ModuleResolutionReport resolve() {
BundleException be = (BundleException) e.getCause();
if (be.getType() == BundleException.REJECTED_BY_HOOK) {
return new ModuleResolutionReport(null, Collections.emptyMap(),
new ResolutionException(be));
new ResolutionException(be), -1, -1, -1, -1, -1);
}
}
throw e;
Expand Down Expand Up @@ -962,7 +993,7 @@ ModuleResolutionReport resolve() {
if (DEBUG_WIRING) {
printWirings(result);
}
report = reportBuilder.build(result, re);
report = reportBuilder.build(result, re, logger);
if (DEBUG_REPORT) {
if (report.getResolutionException() != null) {
Debug.printStackTrace(report.getResolutionException());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.Map.Entry;
import java.util.concurrent.atomic.AtomicBoolean;

import org.apache.felix.resolver.ResolverImpl.PermutationType;
import org.apache.felix.resolver.ResolverImpl.ResolveSession;
import org.apache.felix.resolver.reason.ReasonException;
import org.apache.felix.resolver.util.*;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,29 @@ public void logUsesConstraintViolation(Resource resource, ResolutionError error)
{
// do nothing by default
}

/**
* Called whenever a new permutation is added by the resolver.
*
* @param type the type of the permutation
* @param remaining a function that can be used to query the now current number
* of permutation types
*/
public void logPermutationAdded(PermutationType type)
{
// do nothing by default
}

/**
* Called whenever a permutation is removed and about to be processed by the
* resolver.
*
* @param type the type of permutation that will be processed
* @param remaining a function that can be used to query the now current number
* of permutation types
*/
public void logProcessPermutation(PermutationType type)
{
// do nothing by default
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.felix.resolver;
public enum PermutationType {
USES,
IMPORT,
SUBSTITUTE
}
Loading

0 comments on commit d5e0f49

Please sign in to comment.