Re: Maven Commit 3b671d07340b002e13775883d09e7f7e0d9a3c49
Hi Kristian, There had to be some kind of issue with collection if Karl made IdentityMap synchronized. Whatever collections are too open. As an example, this is not right and it is not right in multithreading either: request.setArtifactDependencies( project.getDependencyArtifacts() ); //common instance shared now in two object where one can change the status of second These collections are internally modifiable, and that's I guess what Herve means in "but inside core: it's harder to define the limits of collaboration" https://github.com/apache/maven/pull/34#issuecomment-68425530 Who is setting and modifying is safe? Who is getting collection and modifying is safe? Where is the limit, which status of object is kept and which one is modified? This is not clear because the impl is too open inside of the core and therefore any object can change anything. Cheers Tibor -- View this message in context: http://maven.40175.n5.nabble.com/Maven-Commit-3b671d07340b002e13775883d09e7f7e0d9a3c49-tp5821660p5821915.html Sent from the Maven Developers mailing list archive at Nabble.com. - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: Maven Commit 3b671d07340b002e13775883d09e7f7e0d9a3c49
We try to move the code forwards, but old code bases are "interesting"; you have to make sure you keep track of your goals. What I am trying to say is that when I think of rouge threads writing in unsynchronized data; I immediately get flashbacks of the gunman in pulp fiction (Samuel L Jackson I believe) who appropriately misquotes bible statements before killing people. This is the guy you're bringing into your build. I just tried to be entirely clear about the threading model that is actually in effect in maven core, which actually works. If your mojo starts writing to shared state from a different thread all hell will be loose. If you do it from the correct thread, things should work. If the problem is present when the mojo threads correctly, I'm all in favour of fixing the problem. If the problem occurs because the mojo violates the threading contract, I'm kind of luke-warm about fixing the issue, since you will open the floodgates of mordor and all the evilness of the old world will come seeping through the cracks. But a fully concurrent maven core that does away with the one mojo one thread model is a huge undertaking, and probably not worth the effort. We had quite a few interesting discussions on optimizing performance. Right now I am working on fully concurrent zip creation in commons-compress, this should come for "free" to all of maven in next plexus-archiver release. There is also stuff like the excellent discussions I've been having with Vladimir Sitnikov in http://jira.codehaus.org/browse/MSHARED-394. Fixing that should have a huge pontential upside. Other than that; build performance is mostly about compiler-plugin and tests. Kristian 2014-12-31 12:42 GMT+01:00 Tibor Digana : > Don't worry Kristian, you did great job in parallel Maven exec, honestly I > don't say opposite! > As the CPUs speed up and disks as well, we really need parallal executions > in Maven core. > The thing how the discussion started in emails is that i wrote some local > changes and opened PR > https://github.com/apache/maven/pull/34 > > IMHO the Maven is getting older and compettitors like Graddle will have > chance to get more users than Maven. > The thing is to keep current Maven users and plugin vendors and make > "stable" Maven. > Also we should introduce what people like, means sciptable languages in POM > like BashShell, Groovy. > > I guess compiling Maven in Java8 no user has benefit of unless using > Java-like language in POM. > Some plugins attempt to do so already. > I am aware of the strong argument of not-cracking POM structures with code > and the idea of configuring projects on regular basis, but there should some > ballance between. > > > > -- > View this message in context: > http://maven.40175.n5.nabble.com/Maven-Commit-3b671d07340b002e13775883d09e7f7e0d9a3c49-tp5821660p5821839.html > Sent from the Maven Developers mailing list archive at Nabble.com. > > - > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > For additional commands, e-mail: dev-h...@maven.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: Maven Commit 3b671d07340b002e13775883d09e7f7e0d9a3c49
Don't worry Kristian, you did great job in parallel Maven exec, honestly I don't say opposite! As the CPUs speed up and disks as well, we really need parallal executions in Maven core. The thing how the discussion started in emails is that i wrote some local changes and opened PR https://github.com/apache/maven/pull/34 IMHO the Maven is getting older and compettitors like Graddle will have chance to get more users than Maven. The thing is to keep current Maven users and plugin vendors and make "stable" Maven. Also we should introduce what people like, means sciptable languages in POM like BashShell, Groovy. I guess compiling Maven in Java8 no user has benefit of unless using Java-like language in POM. Some plugins attempt to do so already. I am aware of the strong argument of not-cracking POM structures with code and the idea of configuring projects on regular basis, but there should some ballance between. -- View this message in context: http://maven.40175.n5.nabble.com/Maven-Commit-3b671d07340b002e13775883d09e7f7e0d9a3c49-tp5821660p5821839.html Sent from the Maven Developers mailing list archive at Nabble.com. - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: Maven Commit 3b671d07340b002e13775883d09e7f7e0d9a3c49
I'll take a look once I get down from the mountains. What little time I'm not skiing I'm hacking on commons-compress right now. Remember that the general multi-threaded contract of maven is simple and clear; unless clearly stated a mojo is not allowed to call back on the maven core model objects on a different thread. Only the thread that invoked the mojo is allowed to write to shared state. I can't really see us leaving this model any time soon, plugin code should really obey this contract. So thread handoffs should be within the mojo. This means that within core we have clear ideas of what code actually sees multiple threads; hopefully :) Kristian 2014-12-30 14:44 GMT+01:00 Tibor Digana : > An excellent operation is CAS (Compare and Swap/Set). This would mean that > POJO would have getters which return "unmodifiableCollection", and new CAS > operations, for instance for the project dependencies. > > The new method would be then: > > public boolean casDependencies(Collection original, > Collection modified). > > The usecase would be then in whatever plugin: > > Collection newDeps; > do { > Collection deps = project.getDependencies(); > newDeps = copyAndModifyDependencies( deps ); > } while ( !project.casDependencies( deps, newDeps ) ); > > Every injectable POJO object will share single guardian lock object per POM > project (plugin has no notion) and use it in CAS operations (maven-core > impl). > > > > -- > View this message in context: > http://maven.40175.n5.nabble.com/Maven-Commit-3b671d07340b002e13775883d09e7f7e0d9a3c49-tp5821660p5821741.html > Sent from the Maven Developers mailing list archive at Nabble.com. > > - > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > For additional commands, e-mail: dev-h...@maven.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: Maven Commit 3b671d07340b002e13775883d09e7f7e0d9a3c49
An excellent operation is CAS (Compare and Swap/Set). This would mean that POJO would have getters which return "unmodifiableCollection", and new CAS operations, for instance for the project dependencies. The new method would be then: public boolean casDependencies(Collection original, Collection modified). The usecase would be then in whatever plugin: Collection newDeps; do { Collection deps = project.getDependencies(); newDeps = copyAndModifyDependencies( deps ); } while ( !project.casDependencies( deps, newDeps ) ); Every injectable POJO object will share single guardian lock object per POM project (plugin has no notion) and use it in CAS operations (maven-core impl). -- View this message in context: http://maven.40175.n5.nabble.com/Maven-Commit-3b671d07340b002e13775883d09e7f7e0d9a3c49-tp5821660p5821741.html Sent from the Maven Developers mailing list archive at Nabble.com. - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: Maven Commit 3b671d07340b002e13775883d09e7f7e0d9a3c49
Hey Karl, This is the list of concurrency issues in Maven. http://jira.codehaus.org/browse/MNG-5705?jql=project%20%3D%20MNG%20AND%20status%20%3D%20Open%20AND%20text%20~%20%22parallel%22 The funny thing with such of these issues is that they may not appear in another environments and new may appear over and over again untill all concurrent objects are immutable, or are at least having safe multithread visibility, or they are ideally fully thread-safe. I have fixed MavenProject.class in my private copy, and I have problem because all collections in maven-core couldn't be fixed as I wanted. Our general problem in maven-core is with sharing collections (List/Map) across objects. This means somebody sets the collection instance in POJO and whole Maven core shares the same instance and retrieves the same instance. I understand the Plugins would like to add new Maven properties into a collection, but it does not mean that the Maven itself should do the same. The Mvane code should build POJO in a builder and then never share the collection instances. Example regarding bad Java concurrency implementation is DefaultMavenExecutionResult.class, MavenProject.class, DefaultArtifact.class which are stateful and fully modifiable. Sharing collections across objects. Is this intended? I would like to make a copy of set collection in DefaultArtifact#dependencyTrail, but my problem is with this statement in LifecycleDependencyResolver.java: artifact.setDependencyTrail( resolved.getDependencyTrail() ); Similar problem in DefaultLegacyArtifactCollector.java: resetArtifact.setAvailableVersions( versions ); An example of implementation with good visibility in DefaultArtifact.class is this change i have made: private final Map metadataMap = new ConcurrentHashMap( 0 ); public Collection getMetadataList() { return Collections.unmodifiableCollection( metadataMap.values() ); } public void addMetadataList(...) { ... metadataMap ...} I don't expect semi-sequential ordering here. -- View this message in context: http://maven.40175.n5.nabble.com/Maven-Commit-3b671d07340b002e13775883d09e7f7e0d9a3c49-tp5821660p5821736.html Sent from the Maven Developers mailing list archive at Nabble.com. - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: Maven Commit 3b671d07340b002e13775883d09e7f7e0d9a3c49
I can fix it, but I guess it will be more improvements. Karl, do we have more JIRA bugs regarding concurrency? -- View this message in context: http://maven.40175.n5.nabble.com/Maven-Commit-3b671d07340b002e13775883d09e7f7e0d9a3c49-tp5821660p5821675.html Sent from the Maven Developers mailing list archive at Nabble.com. - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: Maven Commit 3b671d07340b002e13775883d09e7f7e0d9a3c49
HI Tibor, On 12/29/14 3:19 PM, Tibor Digana wrote: It looks like this was concurrency issue. Does it mean that methods #getBuildSummary and #addBuildSummary are called in different threads? The LifeCycleModuleBuilder calls addBuildSummary ()...where i assume it will happen if you define -T 2 on command line... We changed a pure ArrayList with ConcurrentLinkedQueue in surefire:2.18.1 to have "deterministic" behavior. I guess we should do the same in maven core. If you solved a bit of concurrency, other issue may appear next time, I have still the same problem with thread-unsafe classes DefaultMavenExecutionResult and MavenProject. Both don't use final fields and don't use concurrent collections and they instantiate buildSummaries in #addBuildSummary instead of in constructor. DefaultMavenExecutionResult contains: private List topologicallySortedProjects = Collections.emptyList(); which is not thread safe...might be wise to change this... Kind regards Karl Heinz Marbaise - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: Maven Commit 3b671d07340b002e13775883d09e7f7e0d9a3c49
It looks like this was concurrency issue. Does it mean that methods #getBuildSummary and #addBuildSummary are called in different threads? We changed a pure ArrayList with ConcurrentLinkedQueue in surefire:2.18.1 to have "deterministic" behavior. I guess we should do the same in maven core. If you solved a bit of concurrency, other issue may appear next time, I have still the same problem with thread-unsafe classes DefaultMavenExecutionResult and MavenProject. Both don't use final fields and don't use concurrent collections and they instantiate buildSummaries in #addBuildSummary instead of in constructor. -- View this message in context: http://maven.40175.n5.nabble.com/Maven-Commit-3b671d07340b002e13775883d09e7f7e0d9a3c49-tp5821660p5821667.html Sent from the Maven Developers mailing list archive at Nabble.com. - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org