diff options
author | Mengxin Zhu | 2011-11-21 03:39:34 +0000 |
---|---|---|
committer | Mengxin Zhu | 2011-12-02 06:54:23 +0000 |
commit | 82aeb8dee11f4114494fe3dd0ca1097312d2dfd5 (patch) | |
tree | 02d2cbcb2c5afbfe055c6d193b6009fb53668e1f | |
parent | e50188320c5e998c80ee4a4c6a3e60d426336e68 (diff) | |
download | rt.equinox.p2-82aeb8dee11f4114494fe3dd0ca1097312d2dfd5.tar.gz rt.equinox.p2-82aeb8dee11f4114494fe3dd0ca1097312d2dfd5.tar.xz rt.equinox.p2-82aeb8dee11f4114494fe3dd0ca1097312d2dfd5.zip |
Fix 351944 to avoid unnecessary loading artifact repository when doing query, getting artifact descriptors via artifact key.v20111202-0654
Without fixing though simple artifact repository always checks the last modify of artifacts.jar file, however it still cost some time, it will waste a lot of time especially the repository locates on a read-only NFS server.
Signed-off-by: Mengxin Zhu <kane.zhu@windriver.com>
4 files changed, 116 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/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..16f7dc026 --- /dev/null +++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/Bug351944.java @@ -0,0 +1,103 @@ +/******************************************************************************* + * 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.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) { + if (target.exists()) { + target.setWritable(canWrite, false); + 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 |