diff options
5 files changed, 123 insertions, 4 deletions
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java index b365b0376..79970410d 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java @@ -84,6 +84,11 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme * Does this instance of the repository currently hold a lock */ private boolean holdsLock = false; + /** + * Does this instance of the repository can be locked. + * It will be initialized when initializing the location for repository + */ + private Boolean canLock = null; private long cacheTimestamp = 0l; @@ -327,7 +332,8 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme boolean lockAcquired = false; try { - if (canLock()) { + canLock = new Boolean(canLock()); + if (canLock.booleanValue()) { lockAcquired = lockAndLoad(true, new NullProgressMonitor()); if (!lockAcquired) throw new IllegalStateException("Cannot acquire the lock for " + location); //$NON-NLS-1$ @@ -973,6 +979,8 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme desc.setRepository(this); if (updateTimestamp) updateTimestamp(); + if (canLock == null) + canLock = new Boolean(canLock()); } private String getBlobStoreName(String defaultValue) { @@ -1425,11 +1433,11 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme } /** - * Returns true if this instance of SimpleArtifactRepository holds the lock, - * false otherwise. + * Returns true if this instance of SimpleArtifactRepository holds the lock or + * this repository can't be locked at all due to permission, false otherwise. */ private boolean holdsLock() { - return holdsLock; + return (canLock != null && !canLock.booleanValue()) || holdsLock; } /**URIUtil.toURI(location.toURI() diff --git a/bundles/org.eclipse.equinox.p2.tests/META-INF/MANIFEST.MF b/bundles/org.eclipse.equinox.p2.tests/META-INF/MANIFEST.MF index 18bb1d3af..d99aeb78e 100644 --- a/bundles/org.eclipse.equinox.p2.tests/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.equinox.p2.tests/META-INF/MANIFEST.MF @@ -61,6 +61,7 @@ Eclipse-BundleShape: dir Bundle-ActivationPolicy: lazy Import-Package: javax.xml.parsers, org.eclipse.ant.core, + org.eclipse.core.filesystem, org.eclipse.ecf.filetransfer, org.eclipse.equinox.internal.p2.artifact.processors.md5, org.eclipse.equinox.internal.p2.artifact.processors.pack200, diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/AllTests.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/AllTests.java index 74095af3b..47f820527 100644 --- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/AllTests.java +++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/AllTests.java @@ -27,6 +27,7 @@ public class AllTests extends TestCase { suite.addTestSuite(BatchExecuteArtifactRepositoryTest.class); suite.addTestSuite(Bug252308.class); suite.addTestSuite(Bug265577.class); + suite.addTestSuite(Bug351944.class); suite.addTestSuite(CompositeArtifactRepositoryTest.class); suite.addTestSuite(CorruptedJar.class); suite.addTestSuite(FoldersRepositoryTest.class); diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/Bug351944.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/Bug351944.java new file mode 100644 index 000000000..2b9abfe0c --- /dev/null +++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/Bug351944.java @@ -0,0 +1,109 @@ +/******************************************************************************* + * Copyright (c) 2011 Wind River and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Wind River - initial API and implementation + *******************************************************************************/ +package org.eclipse.equinox.p2.tests.artifact.repository; + +import java.io.File; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.*; +import org.eclipse.core.filesystem.*; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.NullProgressMonitor; +import org.eclipse.equinox.p2.core.ProvisionException; +import org.eclipse.equinox.p2.metadata.IArtifactKey; +import org.eclipse.equinox.p2.query.IQueryResult; +import org.eclipse.equinox.p2.repository.artifact.*; +import org.eclipse.equinox.p2.tests.AbstractProvisioningTest; + +public class Bug351944 extends AbstractProvisioningTest { + + File artifactRepoFile = null; + + @Override + protected void setUp() throws Exception { + super.setUp(); + File testData = getTestData("artifact repository", "testData/bug351944"); + artifactRepoFile = getTempFolder(); + copy("Copy to temporary folder", testData, artifactRepoFile); + changeWritePermission(artifactRepoFile, false); + } + + private void changeWritePermission(File target, boolean canWrite) throws CoreException { + if (target.exists()) { + IFileStore fileStore = EFS.getLocalFileSystem().getStore(target.toURI()); + IFileInfo fileInfo = fileStore.fetchInfo(); + fileInfo.setAttribute(EFS.ATTRIBUTE_GROUP_WRITE, canWrite); + fileInfo.setAttribute(EFS.ATTRIBUTE_OWNER_WRITE, canWrite); + fileStore.putInfo(fileInfo, EFS.SET_ATTRIBUTES, new NullProgressMonitor()); + if (target.isDirectory()) { + for (File child : target.listFiles()) + changeWritePermission(child, canWrite); + } + } + } + + @Override + protected void tearDown() throws Exception { + super.tearDown(); + changeWritePermission(artifactRepoFile, true); + delete(artifactRepoFile); + } + + public void testSimpleRepositoryPerformanceOnLoadReadonlyLocalRepository() throws ProvisionException, URISyntaxException { + final URI testRepo = artifactRepoFile.toURI(); + IArtifactRepositoryManager artifactRepositoryManager = getArtifactRepositoryManager(); + IArtifactRepository repo = artifactRepositoryManager.loadRepository(testRepo, new NullProgressMonitor()); + IQueryResult<IArtifactKey> allArtifactKeys = repo.query(ArtifactKeyQuery.ALL_KEYS, new NullProgressMonitor()); + Set<IArtifactKey> keySet = allArtifactKeys.toUnmodifiableSet(); + + Collection<IArtifactRequest> requests = new ArrayList<IArtifactRequest>(); + for (IArtifactKey key : keySet) + requests.add(artifactRepositoryManager.createMirrorRequest(key, repo, null, null)); + + long start = System.currentTimeMillis(); + IArtifactRequest[] toBeRequests = getRequestsForRepository(repo, requests.toArray(new IArtifactRequest[requests.size()])); + long end = System.currentTimeMillis(); + long queryArtifactOneByOne = end - start; + + start = System.currentTimeMillis(); + IArtifactRequest[] toBeRequests2 = getRequestsForRepository2(repo, requests.toArray(new IArtifactRequest[requests.size()])); + end = System.currentTimeMillis(); + long queryAllArtifacts = end - start; + + assertEquals("Test case has problem, not find same requests.", toBeRequests.length, toBeRequests2.length); + assertEquals("Querying artifact key from simple repository has performance issue.", queryAllArtifacts, queryArtifactOneByOne, 10); + } + + /** + * copy from {@link org.eclipse.equinox.internal.p2.engine.DownloadManager} + * @param repository + * @param requestsToProcess + * @return + */ + private IArtifactRequest[] getRequestsForRepository(IArtifactRepository repository, IArtifactRequest[] requestsToProcess) { + ArrayList<IArtifactRequest> applicable = new ArrayList<IArtifactRequest>(); + for (IArtifactRequest request : requestsToProcess) { + if (repository.contains(request.getArtifactKey())) + applicable.add(request); + } + return applicable.toArray(new IArtifactRequest[applicable.size()]); + } + + private IArtifactRequest[] getRequestsForRepository2(IArtifactRepository repository, IArtifactRequest[] requestsToProcess) { + Set<IArtifactKey> keys = repository.query(ArtifactKeyQuery.ALL_KEYS, new NullProgressMonitor()).toSet(); + ArrayList<IArtifactRequest> applicable = new ArrayList<IArtifactRequest>(); + for (IArtifactRequest request : requestsToProcess) { + if (keys.contains(request.getArtifactKey())) + applicable.add(request); + } + return applicable.toArray(new IArtifactRequest[applicable.size()]); + } +} diff --git a/bundles/org.eclipse.equinox.p2.tests/testData/bug351944/artifacts.jar b/bundles/org.eclipse.equinox.p2.tests/testData/bug351944/artifacts.jar Binary files differnew file mode 100755 index 000000000..be6700724 --- /dev/null +++ b/bundles/org.eclipse.equinox.p2.tests/testData/bug351944/artifacts.jar |