Community
Participate
Working Groups
Sorry if I do not use the correct terms sometimes, I am not quite familiar with the P2 dependency management when it comes to programming it. I work on a project having more than 100 eclipse plugins. I was annoyed by the speed of the "Computing target platform..." part of the build, so I have profiled it once with JProfiler. I have figured out, that if multiple eclipse plugins depend on the same maven artifact, then for this artifact multiple "installable unit" is created, which means multiple MD5 hash caculations, which can be expensive if the artifact is big. I have implemented a caching of such "installable units", since it is not really likely, that such artifact changes during a build. Please take a look at my changeset at https://github.com/liptga/org.eclipse.tycho/commit/949eb12af118bef05a11920a8084881bdd9ac0df . If you consider the changes usable, I am ready to contribute those changes as a private person. Please tell me what to do to be able to submit this officially.
any chance you were using an older tycho version? there was an MD5 performance bug 405716 in p2 which I fixed in Kepler.
Originally the improvement was implemented for 0.17.0. Now I have been granted to contribute it as a private person, so I merged it to master. I took a look at the bug you linked. It might be a non relevant fix anymore, but we could test the performance difference somehow. If some artifacts are big, and inside a reactor multiple projects have them as dependency, it may still make a difference.
If you have a sample project that still shows a noticeable performance improvement using 0.21.0-SNAPSHOT, we can consider the patch.
I will try to make up some...
So, I have compared my patched version with the original 0.20.0 version. We have 252 projects, and most of them have dependencies on normal maven artefacts. This comes from our concept, that we have API bundles, which are shared between frontend (RAP) and backend. Those API bundles are the maven artefacts. If I compare a full "clean install" with using 6 threads for parallel building, then: - without my patch, so without caching: 4:24 - with caching: 3:47 If I just make a clean with 6 threads (so that basicly nothing happens after calculating dependencies): - without my patch, so without caching: 1:26 - with caching: 55 seconds So it seems to make actually a quite big difference in our case. Calculating dependencies became almost 33% faster. Please apply the patch :)
(In reply to Gábor Lipták from comment #5) > If I compare a full "clean install" with using 6 threads for parallel > building, then: parallel build is not supported as of now, see bug 380169. Not sure if it helps to use parallel build here if we want to know whether the patch speeds up things. I think parallel build is a different concern. what I was looking for is a sample project that is public, i.e. everybody has access to it and can compare performance building it with and without your patch. You can either attach a sample project here or e.g. publish one on github or similar. Tobias, can you comment on https://github.com/liptga/org.eclipse.tycho/commit/949eb12af118bef05a11920a8084881bdd9ac0df ? >So, I have compared my patched version with the original 0.20.0 version. We >have 252 projects, and most of them have dependencies on normal maven >artefacts. Do you mean you use pomDependencies=consider? Again, a sample project would really help here otherwise it's guesswork for us to reproduce your particular scenario.
(In reply to comment #6) > Tobias, can you comment on > https://github.com/liptga/org.eclipse.tycho/commit/949eb12af118bef05a11920a8084881bdd9ac0df > ? In principal, caching the results of the pom dependency artifact publishing could make sense. Before I do a closer review, I'd like to ask you to first check the conditions of the Contributor License Agreements [1], and if the change meets the requirements of the CLA, to propose it in Gerrit [2] [1] https://wiki.eclipse.org/CLA [2] http://wiki.eclipse.org/Tycho/Contributor_Guide
Hmm. Actually the "clean only" test was single threaded. I also think that the calculation of dependencies is always single threaded. On the other hand we use multiple threads for building, since after the single threaded calculation of dependencies it still makes a huge difference to run tests parallel and compile parallel. We do not have any problem with Tycho and parallel builds. As for setting, in GUI projects we have this setting for Tycho platform: <plugin> <groupId>org.eclipse.tycho</groupId> <artifactId>target-platform-configuration</artifactId> <version>0.20.777</version> <configuration> <appArgLine>-nl en</appArgLine> <pomDependencies>consider</pomDependencies> <dependency-resolution> <optionalDependencies>ignore</optionalDependencies> </dependency-resolution> <environments> <environment> <os>win32</os> <ws>win32</ws> <arch>x86</arch> </environment> <environment> <os>win32</os> <ws>win32</ws> <arch>x86_64</arch> </environment> <environment> <os>linux</os> <ws>gtk</ws> <arch>x86</arch> </environment> <environment> <os>linux</os> <ws>gtk</ws> <arch>x86_64</arch> </environment> </environments> </configuration> </plugin> So we consider pom dependencies. As far as I understand and I have seen, tycho does not use direct dependencies but only the transitive dependencies. Thats why we have a "gui dependencies" pom project, which has a dependency on all API projects. Then the GUI projects have this "gui-dependencies" as dependency. So we have something like this in our dependency tree: GUI Project +gui dependencies ++API project1 ++API project2 ++API project3 This kind of approach can explain why we have a lot of maven artifacts as dependencies for each GUI project.
(In reply to Gábor Lipták from comment #8) > We do not have any problem with Tycho and > parallel builds. the fact that you didn't notice race conditions does not mean something is thread-safe :) just saying
I have read the CLA, and I have no problem with it. How should I accept it? There is no accept button here : https://www.eclipse.org/legal/CLA.php :)
I know. Till now no problems and the performance gain is impressive. So I live with it. As soon as I find some problem, it will be reported :) (In reply to Jan Sievers from comment #9) > (In reply to Gábor Lipták from comment #8) > > We do not have any problem with Tycho and > > parallel builds. > > the fact that you didn't notice race conditions does not mean something is > thread-safe :) just saying
(In reply to Gábor Lipták from comment #10) > I have read the CLA, and I have no problem with it. How should I accept it? > There is no accept button here : https://www.eclipse.org/legal/CLA.php :) https://wiki.eclipse.org/Development_Resources/Contributing_via_Git#Eclipse_Foundation_Contributor_License_Agreement I also updated https://wiki.eclipse.org/Tycho/Contributor_Guide#Legal_Paperwork to provide the link
CLA is ready. What is next? Should I push the changes to gerrit? I have not used gerrit ever. Could you just pull my changeset from GitHub?
(In reply to comment #13) > CLA is ready. What is next? Should I push the changes to gerrit? I have not used > gerrit ever. Could you just pull my changeset from GitHub? Pushing a change to Gerrit is not hard [1] and it saves us from manually verifying the CLA and running the tests. So please propose your change in this way. [1] https://wiki.eclipse.org/Tycho/Contributor_Guide#Pushing_to_Gerrit
After a bit of suffering with ChangeID and Signing I guess I pushed the changeset. Please take a look at that. Rebased the changeset on current master.
proposed change is https://git.eclipse.org/r/#/c/33660/
Hi Tobias, I applied your suggestions. Caching is now moved to a separate "service" class, and the cache key is now based only the information delivered by the IArtifactFacade. Proposed change: https://git.eclipse.org/r/#/c/42006/ It is based on the master from yesterday. Regards, Gábor
Hmm. There is a test failure on your Hudson server. It has built locally fine. I will take a look at that.
There is a problem in integration tests. I will check it as soon as I have some time.
Gabor, when looking at your change, I wondered why you were caching installable units when you wanted to avoid the MD5 computation. The MD5 sums are not stored in the installable units, but in the artifact descriptors, i.e. the entries in the artifact repository. Of course, when you skip the publishing altogether (due to a cache hit), there is obviously also no artifact repository entry written and hence no MD5 sum computed. However this is also the issue with the proposed patch: The cache prevents side effects on the object returned by TargetPlatformBundlePublisher(Service).getArtifactRepoOfPublishedBundles. To solve this problem, we'd need to have this method always use the same object for the entire reactor instead of returning a separate instance per reactor project (which would be semantically also correct). With such a global PublishedBundlesArtifactRepository instance however, there would then be a simpler solution to avoid the MD5 computation. I've proposed this solution here: https://git.eclipse.org/r/#/c/43595/ It doesn't quite work yet, i.e. it doesn't avoid the MD5 re-computation * because the PublishedBundlesArtifactRepository instance is not global yet, and * there is a small p2 bug which still would need to be fixed: bug 461851 I'd prefer that we try this simpler solution first, and only if this doesn't solve the performance issue, go for the more complicated bundles publisher cache solution.
Tobias, My proposed solution was simply driven by profiling. I have just found the slow parts and tried to find a point in the code to improve it. Since I do not know Tycho and Maven codebase obviously it is quite difficult for me to understand the architecture (which objects are singletons and so on). If you think so, of course we can wait for the other things to be fixed. Meanwhile we can park this ticket. Regards, Gábor
The linked p2 bug won't be fixed unless we from the Tycho project fix it. But this shouldn't be a problem because I'm also committer on the p2 project. So if you want, you could have a look at the p2 bug and see if you can provide a patch. Otherwise, I'll probably also look into this eventually.
Eclipse Tycho is moving away from this bugs.eclipse.org issue tracker to https://github.com/eclipse/tycho/issues/ instead. If this issue is relevant to you, your action is required. 0. Verify this issue is still happening with latest Tycho 2.4.0-SNAPSHOT if issue has disappeared, please change status of this issue to "CLOSED WORKFORME" with some details about your testing environment and how you did verify the issue; and you're done if issue is still present when latest release: * Create a new issue at https://github.com/eclipse/tycho/issues/ ** Use as title in GitHub the title of this Bugzilla ticket (may include the bug number or not, at your own convenience) ** In the GitHub description, start with a link to this bugzilla ticket ** Optionally add new content to the description if it can helps towards resolution ** Submit GitHub issue * Update bugzilla ticket ** Add to "See also" property (up right column) the link to the newly created GitHub issue ** Add a comment "Migrated to <link-to-newly-created-GitHub-issue>" ** Set status as CLOSED MOVED ** Submit All issues that remain open will be automatically closed next week or so. Then the Bugzilla component for Tycho will be archived and made read-only.
