[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348585346

   > If we ignore the figures right - highly depends your build, tomee 
container/openejb-core module for ex, it is 14s -> 4s (1 module so if you do 
the math you almost save 1h for your example ;)).
   
   Sure, we can come up with numbers as we like, but I will shut up in a moment 
this very feature saves 1h on a _real life project_ out there.
   
   > Kind of, "magic" happens there 
https://github.com/apache/maven-compiler-plugin/blob/master/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java#L897
 and agree the rule can be enhanced to better manage multi-reactor builds, can 
be a great enhancement/PR to do.
   
   Sure, but this also means that almost no multi module build benefits from 
this feature, as the sole idea of multi module build is usually to "depend on 
each other". I can imagine some "lab" project where you have 300 modules that 
do not depend on each other, but I guess this cannot be called "majority", 
unless am mistaken.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348510150

   Recap what we have so fat:
   * "incremental" feature may bring 4 minute gain on 300 module build, unless
   * there are inter-module dependencies, as in that case this feature would 
work only on "root" modules?
   
   Is this right?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348501492

   > @cstamas you mean slower and produces the same output? master does by 
default a full rebuild in 10s whereas your PR does a full rebuild in 14s
   
   The I don't understand this: "17s for the first build, 10s for the second", 
isn't first the "full"?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348488063

   Ok, tried with another project, Maven this time:
   
   maven master: 984f73dc7ca2515ee520fbded3382820f62116e9
   
   # Invocations
   
   1st: `mvn clean install -DskipTests`
   2nd: `mvn verify -DskipTests`
   
   # With master
   
   1st: 49.868s
   2nd: 36.577s
   
   # With PR
   
   1st: 48.978s
   2nd: 41.383s
   
   And again, the feature did not work (!) as I understand, as there are 
inter-dependencies in reactor? 
   
   What's the point of multi module build if this feature does NOT work with 
it? Am I right that this feature works on multi module build ONLY if there are 
no inter project dependencies? I just realized what @olamy was saying


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348457823

   @rmannibucau ok, thanks for doing this. As I thought: PR is _clearly better_ 
at "full recompile" case of m-c-p master


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348422749

   @rmannibucau could you please build locally this PR and try with that same 
steps?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348406757

   @slawekjaranowski please compare m-c-p master (or any released) vs this PRl 
:smile:  As your test really compares m-c-p against m-c-p, not m-c-p 
master/released against PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348400764

   @rmannibucau a bit of simple math:
   your "full" compile: 1391ms
   your "incremental" compile: 572ms
   difference is: 819ms
   On "big" build (a 300 module build was mentioned): 300 x 819ms = 245700ms = 
245,7s = 4,095min
   
   So, the diff you showed _would_ produce on 300 module build total time 
difference of 4 minutes? Am interested in how much percentage these 4 minutes 
are of the full build time10%? 1%? 0.05%? Let's find out!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348361295

   Full logs of `mvn verify -DskipTests` are here (they are huge) 
https://drive.google.com/file/d/1S8GUWW7C2SC2Wv4_LjxCji_VaJZORGMj/view?usp=share_link


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348309636

   @rmannibucau i hope you understand your example does not make sense? Can you 
show me a real life example, a full build?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348251135

   Nope, I removed bits needing install, 3rd invocation is `mvn verify`, please 
reread.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-13 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1348211764

   As everyone talks how this is "must to have" but nobody provided any number 
to support this claim, I tried to convince myself this is "must to have". So...
   
   Subject build: 
   Jetty branch `jetty-10.0.x` (6e82e70edf6a106b9fd08ccfacd840efef8a95f9) with 
172 modules from https://github.com/eclipse/jetty.project
   
   ## Preparations
   
   Built this checked out project "as usual" (by the book) using `-DskipTests` 
as README recommends to prime local repo. After local repo "primed" (to have no 
remote fetch happens), spotted that asciidoctor-maven-plugin always goes 
remote, so removed documentation module that uses it, to prevent huge variance 
(as it goes directly to central, not local MRM).
   
   Then, as 2nd (w/o clean) invocation consistently failed, had to remove 
tests-integration module as well, to be able to test "incremental" use case.
   
   This leaves us with 170 modules.
   
   Finally,. changed jetty to use locally built 3.11.0-SNAPSHOT of 
maven-compile-plugin:
   * master (5be316391ea7c575ee65c3fe83b4c7b30e1174d0) 
   * vs PR (7b59074b400974b57e9c4cd84269acd39d7a7f9a)
   
   Full diff of my local changes:
   https://gist.github.com/cstamas/b637f4bce0967836cf835cf21e105ab9
   
   ## Used Invocations
   
   I used following 3 invocations to time:
   1st invocation: `mvn clean install -DskipTests`
   2nd invocation: `mvn install -DskipTests`
   3rd invocation: `mvn verify -DskipTests`
   
   Used Maven "total time" output as measure.
   
   ## With m-c-p:master
   
   1st master: 02:10 min
   2nd master: 01:39 min
   3rd master: 01:35 min
   
   ## With m-c-p:PR
   
   1st PR: 02:11 min
   2nd PR: 01:35 min
   3rd PR: 01:36 min
   
   ## Conclusion
   
   Please, somebody, anybody, repeat these (the numbers cannot be "cross" 
compared, only can be compared among runs on same HW/env), and as always, "this 
is not a proper benchmark" type of notice... but.
   
   So far, it seems Jetty project (170) modules is not "large" enough to 
produce noticeable difference?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-12 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1347397562

   As expected, without the two IT it all pass OK.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-compiler-plugin] cstamas commented on pull request #160: [MCOMPILER-515] This plugin is not "incremental"

2022-12-12 Thread GitBox


cstamas commented on PR #160:
URL: 
https://github.com/apache/maven-compiler-plugin/pull/160#issuecomment-1347354841

   As expected, the two ITs did fail on GH CI.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org