Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Watson2018-11-07 22:30:30 +0000
committerThomas Watson2018-11-16 16:21:12 +0000
commit04612c2b5b2ad02cefce7d96e625355298273088 (patch)
tree82b8d6d41c8d999c9dba124b3df6358476d5b2d6
parent6043d928c30679720e0b3970754a470db25a9ec6 (diff)
downloadrt.equinox.framework-Y20181121-2200.tar.gz
rt.equinox.framework-Y20181121-2200.tar.xz
rt.equinox.framework-Y20181121-2200.zip
Massive number of resolving threads causes high levels of lock/retry contention, so we limit the number of concurrent resolves to 10 Change-Id: I58e4a704f64e824573dfd9e9f5b32f6802d8982c Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
-rwxr-xr-xbundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java98
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java278
2 files changed, 269 insertions, 107 deletions
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java
index 7009ee5af..67607a729 100755
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java
@@ -2855,6 +2855,20 @@ public class SystemBundleTests extends AbstractBundleTests {
return bundles;
}
+ private static File[] createBundles(File outputDir, int bundleCount, Map<String, String> extraHeaders) throws IOException {
+ outputDir.mkdirs();
+
+ File[] bundles = new File[bundleCount];
+
+ for (int i = 0; i < bundleCount; i++) {
+ Map<String, String> headers = new HashMap<>(extraHeaders);
+ headers.put(Constants.BUNDLE_SYMBOLICNAME, "-b" + i);
+ bundles[i] = createBundle(outputDir, "-b" + i, headers);
+ }
+
+ return bundles;
+ }
+
public static File createBundle(File outputDir, String id, boolean emptyManifest, boolean dirBundle) throws IOException {
File file = new File(outputDir, "bundle" + id + (dirBundle ? "" : ".jar")); //$NON-NLS-1$ //$NON-NLS-2$
if (!dirBundle) {
@@ -3856,4 +3870,88 @@ public class SystemBundleTests extends AbstractBundleTests {
}
}
}
+
+ // Note this is more of a performance test. It has a timeout that will cause it to
+ // fail if it takes too long.
+ public void testMassiveParallelInstallStart() {
+ File config = OSGiTestsActivator.getContext().getDataFile(getName()); //$NON-NLS-1$
+ Map<String, Object> configuration = new HashMap<String, Object>();
+ configuration.put(Constants.FRAMEWORK_STORAGE, config.getAbsolutePath());
+
+ final Equinox equinox = new Equinox(configuration);
+ try {
+ equinox.init();
+ } catch (BundleException e) {
+ fail("Unexpected exception in init()", e); //$NON-NLS-1$
+ }
+ // should be in the STARTING state
+ assertEquals("Wrong state for SystemBundle", Bundle.STARTING, equinox.getState()); //$NON-NLS-1$
+ final BundleContext systemContext = equinox.getBundleContext();
+ assertNotNull("System context is null", systemContext); //$NON-NLS-1$
+ try {
+ equinox.start();
+ } catch (BundleException e) {
+ fail("Failed to start the framework", e); //$NON-NLS-1$
+ }
+ assertEquals("Wrong state for SystemBundle", Bundle.ACTIVE, equinox.getState()); //$NON-NLS-1$
+
+ long startCreateBundle = System.nanoTime();
+ final int numBundles = 2000;
+ final File[] testBundles;
+ try {
+ testBundles = createBundles(new File(config, "bundles"), numBundles, Collections.singletonMap(Constants.DYNAMICIMPORT_PACKAGE, "*")); //$NON-NLS-1$
+ } catch (IOException e) {
+ fail("Unexpected error creating budnles", e); //$NON-NLS-1$
+ throw new RuntimeException();
+ }
+ System.out.println("Time to create: " + TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startCreateBundle));
+
+ long startReslveTime = System.nanoTime();
+ ExecutorService executor = Executors.newFixedThreadPool(50);
+ final List<Exception> errors = new CopyOnWriteArrayList<>();
+ try {
+ for (final File f : testBundles) {
+ executor.execute(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ String location = f.toURI().toURL().toExternalForm();
+ System.out.println("Installing: " + f.getName());
+ Bundle b = systemContext.installBundle(location);
+ b.start();
+ BundleWiring wiring = b.adapt(BundleWiring.class);
+ wiring.getClassLoader().loadClass(BundleContext.class.getName());
+ } catch (Exception e) {
+ errors.add(e);
+ }
+ }
+ });
+ }
+ } finally {
+ executor.shutdown();
+ try {
+ assertTrue("Operation took too long", executor.awaitTermination(5, TimeUnit.MINUTES));
+ } catch (InterruptedException e) {
+ fail("Interrupted.", e);
+ }
+ }
+ System.out.println("Time to resolve: " + TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startReslveTime));
+
+ Assert.assertEquals("Errors found.", Collections.emptyList(), errors);
+ Assert.assertEquals("Wrong number of bundles.", numBundles + 1, systemContext.getBundles().length);
+
+ List<BundleWire> providedWires = equinox.adapt(BundleWiring.class).getProvidedWires(PackageNamespace.PACKAGE_NAMESPACE);
+ Assert.assertEquals("Wrong number of provided wires.", numBundles, providedWires.size());
+
+ try {
+ equinox.stop();
+ } catch (BundleException e) {
+ fail("Unexpected erorr stopping framework", e); //$NON-NLS-1$
+ }
+ try {
+ equinox.waitForStop(10000);
+ } catch (InterruptedException e) {
+ fail("Unexpected interrupted exception", e); //$NON-NLS-1$
+ }
+ }
}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java
index 8bf37cd79..73bfac5bb 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java
@@ -13,6 +13,7 @@
*******************************************************************************/
package org.eclipse.osgi.container;
+import java.io.Closeable;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.ArrayList;
@@ -25,9 +26,11 @@ import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.ReentrantLock;
import org.eclipse.osgi.container.Module.StartOptions;
import org.eclipse.osgi.container.Module.State;
import org.eclipse.osgi.container.Module.StopOptions;
@@ -482,19 +485,23 @@ public final class ModuleContainer implements DebugOptionsListener {
return new ModuleResolutionReport(null, Collections.<Resource, List<Entry>> emptyMap(), new ResolutionException("Unable to resolve while shutting down the framework.")); //$NON-NLS-1$
}
ResolutionReport report = null;
- do {
- try {
- report = resolveAndApply(triggers, triggersMandatory, restartTriggers);
- } catch (RuntimeException e) {
- if (e.getCause() instanceof BundleException) {
- BundleException be = (BundleException) e.getCause();
- if (be.getType() == BundleException.REJECTED_BY_HOOK || be.getType() == BundleException.STATECHANGE_ERROR) {
- return new ModuleResolutionReport(null, Collections.<Resource, List<Entry>> emptyMap(), new ResolutionException(be));
+ try (ResolutionLock resolutionLock = new ResolutionLock(1)) {
+ do {
+ try {
+ report = resolveAndApply(triggers, triggersMandatory, restartTriggers);
+ } catch (RuntimeException e) {
+ if (e.getCause() instanceof BundleException) {
+ BundleException be = (BundleException) e.getCause();
+ if (be.getType() == BundleException.REJECTED_BY_HOOK || be.getType() == BundleException.STATECHANGE_ERROR) {
+ return new ModuleResolutionReport(null, Collections.<Resource, List<Entry>> emptyMap(), new ResolutionException(be));
+ }
}
+ throw e;
}
- throw e;
- }
- } while (report == null);
+ } while (report == null);
+ } catch (ResolutionLockException e) {
+ return new ModuleResolutionReport(null, Collections.<Resource, List<Entry>> emptyMap(), new ResolutionException("Timeout acquiring lock for resolution", e, Collections.<Requirement> emptyList())); //$NON-NLS-1$
+ }
return report;
}
@@ -553,90 +560,94 @@ public final class ModuleContainer implements DebugOptionsListener {
Map<ModuleRevision, ModuleWiring> deltaWiring;
Collection<Module> modulesResolved;
long timestamp;
- do {
- result = null;
- Map<ModuleRevision, ModuleWiring> wiringClone = null;
- List<DynamicModuleRequirement> dynamicReqs = null;
- Collection<ModuleRevision> unresolved = new ArrayList<>();
- moduleDatabase.readLock();
- try {
- ModuleWiring wiring = revision.getWiring();
- if (wiring == null) {
- // not resolved!!
- return null;
- }
- if (wiring.isDynamicPackageMiss(dynamicPkgName)) {
- // cached a miss for this package
- return null;
- }
- // need to check that another thread has not done the work already
- result = findExistingDynamicWire(revision.getWiring(), dynamicPkgName);
- if (result != null) {
- return result;
- }
- dynamicReqs = getDynamicRequirements(dynamicPkgName, revision);
- if (dynamicReqs.isEmpty()) {
- // save the miss for the package name
- wiring.addDynamicPackageMiss(dynamicPkgName);
- return null;
- }
- timestamp = moduleDatabase.getRevisionsTimestamp();
- wiringClone = moduleDatabase.getWiringsClone();
- Collection<Module> allModules = moduleDatabase.getModules();
- for (Module module : allModules) {
- ModuleRevision current = module.getCurrentRevision();
- if (current != null && !wiringClone.containsKey(current))
- unresolved.add(current);
+ try (ResolutionLock resolutionLock = new ResolutionLock(MAX_RESOLUTION_PERMITS)) {
+ do {
+ result = null;
+ Map<ModuleRevision, ModuleWiring> wiringClone = null;
+ List<DynamicModuleRequirement> dynamicReqs = null;
+ Collection<ModuleRevision> unresolved = new ArrayList<>();
+ moduleDatabase.readLock();
+ try {
+ ModuleWiring wiring = revision.getWiring();
+ if (wiring == null) {
+ // not resolved!!
+ return null;
+ }
+ if (wiring.isDynamicPackageMiss(dynamicPkgName)) {
+ // cached a miss for this package
+ return null;
+ }
+ // need to check that another thread has not done the work already
+ result = findExistingDynamicWire(revision.getWiring(), dynamicPkgName);
+ if (result != null) {
+ return result;
+ }
+ dynamicReqs = getDynamicRequirements(dynamicPkgName, revision);
+ if (dynamicReqs.isEmpty()) {
+ // save the miss for the package name
+ wiring.addDynamicPackageMiss(dynamicPkgName);
+ return null;
+ }
+ timestamp = moduleDatabase.getRevisionsTimestamp();
+ wiringClone = moduleDatabase.getWiringsClone();
+ Collection<Module> allModules = moduleDatabase.getModules();
+ for (Module module : allModules) {
+ ModuleRevision current = module.getCurrentRevision();
+ if (current != null && !wiringClone.containsKey(current))
+ unresolved.add(current);
+ }
+ } finally {
+ moduleDatabase.readUnlock();
}
- } finally {
- moduleDatabase.readUnlock();
- }
- deltaWiring = null;
- boolean foundCandidates = false;
- for (DynamicModuleRequirement dynamicReq : dynamicReqs) {
- ModuleResolutionReport report = moduleResolver.resolveDynamicDelta(dynamicReq, unresolved, wiringClone, moduleDatabase);
- Map<Resource, List<Wire>> resolutionResult = report.getResolutionResult();
- deltaWiring = resolutionResult == null ? Collections.<ModuleRevision, ModuleWiring> emptyMap() : moduleResolver.generateDelta(resolutionResult, wiringClone);
- if (deltaWiring.get(revision) != null) {
- break;
- }
- // Did not establish a valid wire.
- // Check to see if any candidates were available.
- // this is used for caching purposes below
- List<Entry> revisionEntries = report.getEntries().get(revision);
- if (revisionEntries == null || revisionEntries.isEmpty()) {
- foundCandidates = true;
- } else {
- // must make sure there is no MISSING_CAPABILITY type entry
- boolean isMissingCapability = false;
- for (Entry entry : revisionEntries) {
- isMissingCapability |= Entry.Type.MISSING_CAPABILITY.equals(entry.getType());
+ deltaWiring = null;
+ boolean foundCandidates = false;
+ for (DynamicModuleRequirement dynamicReq : dynamicReqs) {
+ ModuleResolutionReport report = moduleResolver.resolveDynamicDelta(dynamicReq, unresolved, wiringClone, moduleDatabase);
+ Map<Resource, List<Wire>> resolutionResult = report.getResolutionResult();
+ deltaWiring = resolutionResult == null ? Collections.<ModuleRevision, ModuleWiring> emptyMap() : moduleResolver.generateDelta(resolutionResult, wiringClone);
+ if (deltaWiring.get(revision) != null) {
+ break;
+ }
+ // Did not establish a valid wire.
+ // Check to see if any candidates were available.
+ // this is used for caching purposes below
+ List<Entry> revisionEntries = report.getEntries().get(revision);
+ if (revisionEntries == null || revisionEntries.isEmpty()) {
+ foundCandidates = true;
+ } else {
+ // must make sure there is no MISSING_CAPABILITY type entry
+ boolean isMissingCapability = false;
+ for (Entry entry : revisionEntries) {
+ isMissingCapability |= Entry.Type.MISSING_CAPABILITY.equals(entry.getType());
+ }
+ foundCandidates |= !isMissingCapability;
}
- foundCandidates |= !isMissingCapability;
}
- }
- if (deltaWiring == null || deltaWiring.get(revision) == null) {
- if (!foundCandidates) {
- ModuleWiring wiring = revision.getWiring();
- if (wiring != null) {
- // save the miss for the package name
- wiring.addDynamicPackageMiss(dynamicPkgName);
+ if (deltaWiring == null || deltaWiring.get(revision) == null) {
+ if (!foundCandidates) {
+ ModuleWiring wiring = revision.getWiring();
+ if (wiring != null) {
+ // save the miss for the package name
+ wiring.addDynamicPackageMiss(dynamicPkgName);
+ }
}
+ return null; // nothing to do
}
- return null; // nothing to do
- }
- modulesResolved = new ArrayList<>();
- for (ModuleRevision deltaRevision : deltaWiring.keySet()) {
- if (!wiringClone.containsKey(deltaRevision))
- modulesResolved.add(deltaRevision.getRevisions().getModule());
- }
+ modulesResolved = new ArrayList<>();
+ for (ModuleRevision deltaRevision : deltaWiring.keySet()) {
+ if (!wiringClone.containsKey(deltaRevision))
+ modulesResolved.add(deltaRevision.getRevisions().getModule());
+ }
- // Save the result
- ModuleWiring wiring = deltaWiring.get(revision);
- result = findExistingDynamicWire(wiring, dynamicPkgName);
- } while (!applyDelta(deltaWiring, modulesResolved, Collections.<Module> emptyList(), timestamp, false));
+ // Save the result
+ ModuleWiring wiring = deltaWiring.get(revision);
+ result = findExistingDynamicWire(wiring, dynamicPkgName);
+ } while (!applyDelta(deltaWiring, modulesResolved, Collections.<Module> emptyList(), timestamp, false));
+ } catch (ResolutionLockException e) {
+ return null;
+ }
return result;
}
@@ -660,19 +671,69 @@ public final class ModuleContainer implements DebugOptionsListener {
return null;
}
- private final Object stateLockMonitor = new Object();
+ // The resolution algorithm uses optimistic locking approach;
+ // This involves taking a snapshot of the state and performing an
+ // operation on the snapshot while holding no locks and then
+ // obtaining the write lock to apply the results. If we
+ // detect the state has changed since the snapshot taken then
+ // the process is started over. If we allow too many threads
+ // to try to do this at the same time it causes thrashing
+ // between taking the snapshot and successfully applying the
+ // results. Instead of resorting to single threaded operations
+ // we choose to limit the number of concurrent resolves
+ final static int MAX_RESOLUTION_PERMITS = 10;
+ final Semaphore _resolutionLock = new Semaphore(MAX_RESOLUTION_PERMITS);
+ final ReentrantLock _bundleStateLock = new ReentrantLock();
+
+ static class ResolutionLockException extends Exception {
+ private static final long serialVersionUID = 1L;
+
+ public ResolutionLockException() {
+ super();
+ }
+
+ public ResolutionLockException(Throwable cause) {
+ super(cause);
+ }
+ }
+
+ class ResolutionLock implements Closeable {
+ private final int permits;
+
+ ResolutionLock(int permits) throws ResolutionLockException {
+ this.permits = permits;
+ boolean previousInterruption = Thread.interrupted();
+ try {
+ if (!_resolutionLock.tryAcquire(permits, 30, TimeUnit.SECONDS)) {
+ throw new ResolutionLockException();
+ }
+ } catch (InterruptedException e) {
+ throw new ResolutionLockException(e);
+ } finally {
+ if (previousInterruption) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ }
+
+ @Override
+ public void close() {
+ _resolutionLock.release(permits);
+ }
+ }
private boolean applyDelta(Map<ModuleRevision, ModuleWiring> deltaWiring, Collection<Module> modulesResolved, Collection<Module> triggers, long timestamp, boolean restartTriggers) {
List<Module> modulesLocked = new ArrayList<>(modulesResolved.size());
// now attempt to apply the delta
try {
- synchronized (stateLockMonitor) {
- // Acquire the necessary RESOLVED state change lock.
- // Note this is done while holding a global lock to avoid multiple threads trying to compete over
- // locking multiple modules; otherwise out of order locks between modules can happen
- // NOTE this MUST be done outside of holding the moduleDatabase lock also to avoid
- // introducing out of order locks between the bundle state change lock and the moduleDatabase
- // lock.
+ // Acquire the necessary RESOLVED state change lock.
+ // Note this is done while holding a global lock to avoid multiple threads trying to compete over
+ // locking multiple modules; otherwise out of order locks between modules can happen
+ // NOTE this MUST be done outside of holding the moduleDatabase lock also to avoid
+ // introducing out of order locks between the bundle state change lock and the moduleDatabase
+ // lock.
+ _bundleStateLock.lock();
+ try {
for (Module module : modulesResolved) {
try {
// avoid grabbing the lock if the timestamp has changed
@@ -690,7 +751,10 @@ public final class ModuleContainer implements DebugOptionsListener {
throw new IllegalStateException(Msg.ModuleContainer_StateLockError, e);
}
}
+ } finally {
+ _bundleStateLock.unlock();
}
+
Map<ModuleWiring, Collection<ModuleRevision>> hostsWithDynamicFrags = new HashMap<>(0);
moduleDatabase.writeLock();
try {
@@ -895,18 +959,19 @@ public final class ModuleContainer implements DebugOptionsListener {
// NOTE this MUST be done outside of holding the moduleDatabase lock also to avoid
// introducing out of order locks between the bundle state change lock and the moduleDatabase
// lock.
- synchronized (stateLockMonitor) {
- try {
- // go in reverse order
- for (ListIterator<Module> iTriggers = refreshTriggers.listIterator(refreshTriggers.size()); iTriggers.hasPrevious();) {
- Module refreshModule = iTriggers.previous();
- refreshModule.lockStateChange(ModuleEvent.UNRESOLVED);
- modulesLocked.add(refreshModule);
- }
- } catch (BundleException e) {
- // TODO throw some appropriate exception
- throw new IllegalStateException(Msg.ModuleContainer_StateLockError, e);
+ _bundleStateLock.lock();
+ try {
+ // go in reverse order
+ for (ListIterator<Module> iTriggers = refreshTriggers.listIterator(refreshTriggers.size()); iTriggers.hasPrevious();) {
+ Module refreshModule = iTriggers.previous();
+ refreshModule.lockStateChange(ModuleEvent.UNRESOLVED);
+ modulesLocked.add(refreshModule);
}
+ } catch (BundleException e) {
+ // TODO throw some appropriate exception
+ throw new IllegalStateException(Msg.ModuleContainer_StateLockError, e);
+ } finally {
+ _bundleStateLock.unlock();
}
// Must not hold the module database lock while stopping bundles
// Stop any active bundles and remove non-active modules from the refreshTriggers
@@ -974,7 +1039,6 @@ public final class ModuleContainer implements DebugOptionsListener {
module.unlockStateChange(ModuleEvent.UNRESOLVED);
}
}
-
// publish unresolved events after giving up all locks
for (Module module : modulesUnresolved) {
adaptor.publishModuleEvent(ModuleEvent.UNRESOLVED, module, module);

Back to the top