From f57a1651a3ed5baa7393d575b7f8726b31707d28 Mon Sep 17 00:00:00 2001 From: Mengxin Zhu Date: Mon, 21 Nov 2011 11:39:34 +0800 Subject: Fix 351944 to avoid unnecessary loading artifact repository when doing query, getting artifact descriptors via artifact key. 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. This version of fix introduces the dependency on o.e.core.filesystem to use EFS API to change the file permission crossing platforms. Signed-off-by: Mengxin Zhu --- .../simple/SimpleArtifactRepository.java | 16 ++- .../META-INF/MANIFEST.MF | 1 + .../p2/tests/artifact/repository/AllTests.java | 1 + .../p2/tests/artifact/repository/Bug351944.java | 109 +++++++++++++++++++++ .../testData/bug351944/artifacts.jar | Bin 0 -> 127964 bytes 5 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/Bug351944.java create mode 100755 bundles/org.eclipse.equinox.p2.tests/testData/bug351944/artifacts.jar 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 allArtifactKeys = repo.query(ArtifactKeyQuery.ALL_KEYS, new NullProgressMonitor()); + Set keySet = allArtifactKeys.toUnmodifiableSet(); + + Collection requests = new ArrayList(); + 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 applicable = new ArrayList(); + 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 keys = repository.query(ArtifactKeyQuery.ALL_KEYS, new NullProgressMonitor()).toSet(); + ArrayList applicable = new ArrayList(); + 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 new file mode 100755 index 000000000..be6700724 Binary files /dev/null and b/bundles/org.eclipse.equinox.p2.tests/testData/bug351944/artifacts.jar differ -- cgit v1.2.3