Community
Participate
Working Groups
Build Identifier: I20110613-1736 A huge performance problem hit my installer, formerly based on 3.6, now migrated to 3.7: A call to calculate needed disk space (using SimpleArti.getArtifactDescriptors(key)) went from 2 seconds to 140 seconds. Debugging the problem showed p2 to try and load the arti repo all the time, since I was installing from a network filesystem, readonly and rather slow, compared to a local disk. Each check for a modification takes 40ms there, which quickly sums up to the huge numbers I was seeing... Looking at the SimlpeArtiRepo implementation I recommend not rechecking the local files with every single call to "getArtifactDescriptors(key)", more generally thinking a read-only arti-repo should probably be trusted to stay the same in-between getArtifactDescriptors(key) , what usecase would be caught by constantly checking the read-only location, other instances messing with the repo? Not sure that would bode well anyway since all information gathered till we find a repo to be modified would be out-of-date, and client codes have no way of detecting that.. Let me know what you think, I think we should at least support the property "eclipse.p2.internal.simple.artifact.repository.locking" throughout the code so that customers like my installer can disable it after finding issues... Reproducible: Always Steps to Reproduce: 1. have a slow, read-only filesystem (nfs for me, on a windows client) 2. in code have a few hundred keys from metadata, query arti repo with getArtifactDescriptors to get the descriptors (and the size as property) 3. calculate time it takes for each call to return (for me 40-60ms)
One way to improve the situation could be to check if the drive is readonly when the repo is loaded and cache this until the repo is unloaded from memory.
(In reply to comment #1) > One way to improve the situation could be to check if the drive is readonly > when the repo is loaded and cache this until the repo is unloaded from memory. All, I've run into the second, terrible performance problem with this change, this time I'm not sure how to best work around it, since it's p2 code the calls the artifact repos(DownloadManager::getRequestsForRepository): <code> for (IArtifactRequest request : requestsToProcess) { if (repository.contains(request.getArtifactKey())) applicable.add(request); } </code> What this does to me is loading my arti repos from a slow disk , times the number of artifacts to install :( This quickly snowballs into a collect phase taking as much as 5 minutes until it even starts to fetch artifacts, which then is done in under a minute. I guess I could derive from the DownloadManager and override the behavior to adapt to the performance bug, but ideally I would like to use a 3.7.x fix, like Pascal is proposing: do not re-load from read-only drives, at least if a certain property is set or not set... Helmut
Can you confirm that the situation you are interested in is one where the drive is really read only?
(In reply to comment #3) > Can you confirm that the situation you are interested in is one where the drive > is really read only? Confirm, the location is a samba share, mounted read-only. It's considered local (no http)... Helmut
Ping, is this something that could be considered for Indigo SR 2 ?
Created attachment 207286 [details] a patch with unit test checking whether the repository can't be locked when initializing the location. If the repository can't be locked at all, it never reloads the repository when doing query and getting artifact keys.
Ian and Pascal, do you have objections for my proposed fix? Thanks.
Released to master. http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=82aeb8dee11f4114494fe3dd0ca1097312d2dfd5
I've reverted this commit for now. Turns out that one of the test cases uses a 1.6 API. Since our compiler compliance is set to 1.5, this caused the build to fail. This should be pretty easy to fix, but because builds have been breaking all day -- and we really need a good build out, I simply reverted it for now.
(In reply to comment #9) > I've reverted this commit for now. Turns out that one of the test cases uses a > 1.6 API. Since our compiler compliance is set to 1.5, this caused the build to > fail. > > This should be pretty easy to fix, but because builds have been breaking all day > -- and we really need a good build out, I simply reverted it for now. Sorry for breaking the build. I'll rework on the fix. How do I subscribe the result of nightly build and unit test of equinox/p2?
(In reply to comment #10) > Sorry for breaking the build. I'll rework on the fix. > How do I subscribe the result of nightly build and unit test of equinox/p2? No worries. Subscribe to the Eclipse releng mailing list [1]. Since this is milestone week, there might not have been a nightly build. This only appeared to be a problem in the test case, and likely pretty easy to fix. However, there were other problems with the build (not our fault), so I thought it was best to revert and not get in the way ;). [1] https://dev.eclipse.org/mailman/listinfo/platform-releng-dev
Released the fix again without using Java 6 API. http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=f57a1651a3ed5baa7393d575b7f8726b31707d28
(In reply to comment #12) > Released the fix again without using Java 6 API. > > http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=f57a1651a3ed5baa7393d575b7f8726b31707d28 Due to the timing (wednesday of Milestone week), I'm going to merge this into our integration branch next week.
It's partially fixed. On windows it still has same problem like before. Bug 366781 reports the issue to check whether a location is read-only or not on Windows.
Bug 366781 is fixed, close it.
The target milestone is empty, for what milestone has this been fixed ? I'd assume Juno M4 but I don't know for sure ... and, would a backport into 3.7.2 be possible ? Thanks, Martin
Indigo SR2 is in phase of RC2. The backport need be approved by PMC.
It seems very late to include this in the 3.7.2 SDK. I'm not even sure we have the available number of committers to review the patch at this point. Meng, I would suggest attending the Equinox/p2 call tomorrow and we can talk. http://wiki.eclipse.org/Equinox_Meeting_Minutes
Adding Tom.
At this point I am not in favor of releasing this for 3.7.2. We can discuss at today's call.
