You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/08/20 06:34:32 UTC
[GitHub] [maven] maximilian-novikov-db opened a new pull request #526: Incremental build and shared cache feature
maximilian-novikov-db opened a new pull request #526:
URL: https://github.com/apache/maven/pull/526
To familiarize yourself with the feature we recommend to start with the provided doc: https://github.com/deutschebank/maven/blob/7d1b8c094f2145909978142a0faa4257a244dc22/Documentation/CACHE.md
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rfscholte commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-902495118
If I recall correclty there were one or two existing classes that needed to be extended to be able to support the cache. Could we move those to a separate PR? That will make reduce the amount of code to validate the rest of the and gives us enough time to experiment with it.
One of the things we need to discuss is whether the rest of the code should be moved to an extension.
With a separate PR we can verify all options.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-902490370
Hi,
Looks very promishing, congrats!
I just have 2 small notes on the overall PR.
1. Since we got a positive vote on mvnd, did you think about making both converging to avoid to have two concurrent impl in the repository at the end? Think it is very doable. Can be worth a thread on the list but at the end I see it very beneficial for maven.
2. The only small technical note I would do right now is that jaxb should probably be replaced by standard maven xml parsing (xpp3 or alike), http client by wagon or other built-in dependencies - long story short no maven-core/pom.xml dep change.
For the charset detection I wonder if something like https://github.com/apache/johnzon/blob/7751e96c4731a16a620a99e1165efac98db30566/johnzon-core/src/main/java/org/apache/johnzon/core/RFC4627AwareInputStreamReader.java#L86 would help and avoid the ENCODING_DETECTOR which looks a bit overkill for text file only.
Hope it makes sense.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] maximilian-novikov-db edited a comment on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
maximilian-novikov-db edited a comment on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-959320629
> @maximilian-novikov-db , how does this work with Idea? Does the IDE use cache? Can I import a maven submodule, or should I always import the whole monorepo in idea?
@Dunemaster
1. Yes, it uses the cache. The know limitation - if you have a code generation in your project, you will need to manually reload the project in Idea after the restoration from the cache, to sync Idea's caches. (literally click Maven/'Reload project' in menu)
2. You can do both ways, you can just import a submodule.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] Tibor17 commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-908227539
Hi Alex,
Excellent work and an elaborat.
Unfortunately the xxHash algorithm is not in the list
https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html
We must guarantee that every java would support it.
T
On Wed, Aug 25, 2021 at 1:05 PM Alex ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In Documentation/CACHE-HOWTO.md
> <https://github.com/apache/maven/pull/526#discussion_r695641759>:
>
> > +Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
> +understand how it works and figure out your optimal config
> +
> +### Minimal config
> +
> +Absolutely minimal config which enables incremental maven with local cache
> +
> +```xml
> +<?xml version="1.0" encoding="UTF-8" ?>
> +<cache xmlns="org:apache:maven:cache:config:v1"
> + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
> + xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
> +
> + <configuration>
> + <enabled>true</enabled>
> + <hashAlgorithm>XX</hashAlgorithm>
>
> [image: benchmark]
> <https://camo.githubusercontent.com/35f343cf91b12df3cc73f30216e26ccda56c8983a62d0714014e0a3e07b63b35/68747470733a2f2f686162726173746f726167652e6f72672f776562742f69792f6f352f7a6a2f69796f357a6a6a36366665723179796d7176672d6163783961756d2e706e67>
> Setup:
>
> - run on Oracle JDK 1.8.0.201 on Windows and Linux
> - use 100 jar files with different size from 3 KB to 90 MB, total size
> 474 MB
> - read in single thread, calculate hash for each file and total
> checksum
> - separate folder for each algorithm to avoid disk cache testing
> - 100 rounds to minimize CPU interference from external loads
> - time in seconds, candle body is 25-75 percentile, shadow is min/max
> values
>
> Results:
>
> - xxHash shows stable time (3-4 sec) on both Windows and Linux
> platforms
> - xxHash with Memory Mapped buffer (XXMM) gives another small gain on
> Linux, useful on CI
> - SHA-256 about 4 times slower with a difference of 10 seconds even on
> this small test
>
> P.S. also tested 5157 files with total size 2.6 GB, difference between
> xxHash and SHA-256 was about 1.5-2 min
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/maven/pull/526#discussion_r695641759>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAH7ER2RHTUYOLVYHGF46E3T6TE53ANCNFSM5CPVYJEA>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>
> .
>
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] AlexanderAshitkin edited a comment on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
AlexanderAshitkin edited a comment on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-903311772
Hi
Thanks a lot for the feedback. The idea of this PR is not to finalize the featurem but to demonstrate curent state, and gather feedback. Realistically looking at the current state, there is an expectedly long way forward to align this feature with all the guidelines and architecure requirements. Though the feedback is reasonable and this is exactly what we are looked for, the implementation in this branch is stable and better to be kept as is. Reworks better to be done atop of that in a smaller incremental steps. To move forward following apporach appears reasonable:
1) Merge this to a feature branch as is as a baseline
2) Find way to safely enable for those who is interested under some "Experimental feature" umbrella (with minimal modifications) and merge it in an official distribution (of course safely and compliantly)
3) Gather and accumulate community feedback to produce a sort of feature roadmap. The ultimate target is integrating it to an official release with all the requirements satisified.
4) Continue work in feature branch to adress p.2 decisions and achive mutual agreement between community members
5) integrate it in an agreed/safe way and remove "Experimental flag"
Small clarifications to review comments:
1) @rmannibucau rework repositories - absolutely reasonable, should be aligned. There are some open question to discuss, like best layout for cache, associated cases - like purging cache, store cache metadata, etc. All this could evolve to repositories design discussion.
2) @rmannibucau Replace with built-in dependencies - reasonable, to be discussed. On use of jaxb - the reasone to use it was schema validation. If modello supports that - could easily be reused. ENCODING_DETECTOR - it supports wide set ofencodings beyond UTF unlike of AwareInputStreamReader. So to be discussed case by case, but in overall agree here.
4) @rfscholte on splitting PR - though technically of course possible, but decomposing seems to be a lot of work by itself. Our idea is to merge this as is to a feature branch and continue with smaller enhancements from there as necessary. They could be tageted to master or to another feature branch.
5) regarding location of the feature - it feels like it should be a seprate module or an extension. Happy to rework in that direction - any guidance is very welcome
6) @hboutemy - we will restore original formatting in core and similar to not add noise in review
From my perspective there are folowing implementation related streams of work:
* Move feature to a separate module/extension safely isolated from core and find a proper way to plug it in safely.
* Align implementation with built-in dependencies (like the mentioned Wagon, Modello, etc)
* Agree proper approach to tests implementation for this feature (like introducing wiremock?) and achieve necessary level of confidence in tests coverage
Thank you
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] hboutemy commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r695930776
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
+
+```xml
+<?xml version="1.0" encoding="UTF-8" ?>
+<cache xmlns="org:apache:maven:cache:config:v1"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
+
+ <configuration>
+ <enabled>true</enabled>
+ <hashAlgorithm>XX</hashAlgorithm>
Review comment:
thank you all for the detailed explanation: I learned something new and interesting today :)
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] hboutemy commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
hboutemy commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-903290931
> If I recall correclty there were one or two existing classes that needed to be extended to be able to support the cache. Could we move those to a separate PR? That will make reduce the amount of code to validate the rest of the and gives us enough time to experiment with it.
> One of the things we need to discuss is whether the rest of the code should be moved to an extension.
> With a separate PR we can verify all options.
from first PR review, most code is put in `maven-core` in a new `org.apache.maven.caching` Java package (and generated jaxb sub-package): I suppose this whole code could me moved to a separate `maven-caching` Maven module...
this caching is injected in `maven-core` through quite a few core classes modifications:
- org.apache.maven.graph.DefaultGraphBuilder
- org.apache.maven.lifecycle.internal.DependencyContext
- org.apache.maven.lifecycle.internal.MojoExecutor
- org.apache.maven.lifecycle.internal.NoResolutionContext
- org.apache.maven.plugin.DefaultBuildPluginManager
- org.apache.maven.plugin.MojoCheker
- org.apache.maven.plugin.MojoExecution
- org.apache.maven.project.DefaultProjectBuilder
- org.apache.maven.project.MavenProject
I fear swiching these updates to core extension will be hard...
on how to learn the code, I don't see the PR being split in multiple PRs, but perhaps the unique commit being split in multiple
one source of noisy modifications (and many new dependencies addition) is using jaxb to read XML config files instead of using our usual Modello
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] hboutemy commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r693520290
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
+
+```xml
+<?xml version="1.0" encoding="UTF-8" ?>
+<cache xmlns="org:apache:maven:cache:config:v1"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
+
+ <configuration>
+ <enabled>true</enabled>
+ <hashAlgorithm>XX</hashAlgorithm>
Review comment:
replace XX with SHA-256 will ease people just copy/pasting
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] qweek commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
qweek commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-910115497
> Hi Alex, Excellent work and an elaborat. Unfortunately the xxHash algorithm is not in the list https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html We must guarantee that every java would support it. T
Used open-source implementation:
[https://github.com/OpenHFT/Zero-Allocation-Hashing](https://github.com/OpenHFT/Zero-Allocation-Hashing/blob/master/src/main/java/net/openhft/hashing/XxHash.java)
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] Dunemaster commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
Dunemaster commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-945747917
@maximilian-novikov-db , how does this work with Idea?
Does the IDE use cache?
Can I import a maven submodule, or should I always import the whole monorepo in idea?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-908245798
@Tibor17 not sure what you meant (https://github.com/apache/maven/pull/526/files#diff-2e20e8e0f47932beae58072a57217d523254d5686c1c1b3dec223e57ac4552f9R29)
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] maximilian-novikov-db edited a comment on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
maximilian-novikov-db edited a comment on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-959320629
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] hboutemy commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r693520397
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
Review comment:
should state to put the content in `.mvn/maven-cache-config.xml` file
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] hboutemy commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
hboutemy commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-915150997
PR merged to branch: thanks a lot for the great contribution
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r695475230
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
+
+```xml
+<?xml version="1.0" encoding="UTF-8" ?>
+<cache xmlns="org:apache:maven:cache:config:v1"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
+
+ <configuration>
+ <enabled>true</enabled>
+ <hashAlgorithm>XX</hashAlgorithm>
Review comment:
Note that xxHash by Yann Collet is non-cryptographic. I don't know whether it makes a difference here. I consider all SHA hashes as abysmally slow compared to xxHash and BLAKE3.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] qweek commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
qweek commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-915996602
@rmannibucau I see what you mean: it will kills few optimizations, generate more objects and probably we need to use proper ByteOrder (default byte buffer order is BIG_ENDIAN and algorithm order is LITTLE_ENDIAN). But yes, it's much cleaner and simpler, thank you for sharing your version.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] AlexanderAshitkin edited a comment on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
AlexanderAshitkin edited a comment on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-903311772
Hi
Thanks a lot for the feedback. The idea of this PR is not to finalize the feature but to demonstrate curent state, and to gather initial feedback to decide on the further plan. Realistically looking at the current state, there is an expectedly long way forward to align this feature with all the guidelines and architecure requirements. Though the feedback is reasonable and this is exactly what we are looked for, the implementation in this branch is stable and better to be kept as is. Reworks in small increments looks easier to move forward. At the moment forward following apporach appears reasonable:
1) Merge this to a feature branch as is as a baseline
2) Find way to safely enable for those who is interested under some "Experimental feature" umbrella (with minimal modifications). Ideally merge it to an official distribution (of course safely and compliantly) under a feature flag which will provide a sort of user consent.
3) Gather and accumulate community feedback to produce a sort of feature roadmap. The ultimate target is integrating it to an official release with all the requirements satisified.
4) Continue work in feature branch to adress p.2 decisions and achive mutual agreement between community members
5) integrate it in an agreed/safe way and remove "Experimental flag"
Small clarifications to review comments:
1) @rmannibucau rework repositories - absolutely reasonable, should be aligned. There are some open question to discuss, like best layout for cache, associated use cases - like purging cache, store cache metadata, etc. All this could evolve to some repositories design decisions. Overall approach was is to minimize changes in existing classes.
2) @rmannibucau Replace with built-in dependencies - reasonable, to be discussed. On use of jaxb - the reasone to use it was schema validation. If modello supports that - could easily be reused. ENCODING_DETECTOR - it supports wide set ofencodings beyond UTF unlike of AwareInputStreamReader. So to be discussed case by case, but in overall agree here.
4) @rfscholte on splitting PR - though technically of course possible, but decomposing seems to be a lot of work by itself and will leave feature in a half state. Our idea is to merge this as is to a feature branch and continue with smaller enhancements from there as necessary. They could be targeted to master or to another feature branch.
5) regarding location of the feature - it feels like it should be a seprate module or an extension. Happy to rework in that direction - guidance here is very welcome
6) @hboutemy - we will restore original formatting in core and similar to not add noise in review
From my perspective there are folowing implementation related streams of work:
* Move feature to a separate module/extension safely isolated from core and find a proper way to plug it in safely.
* Align implementation with built-in dependencies (like the mentioned Wagon, Modello, etc)
* Agree proper approach to tests implementation for this feature (like introducing wiremock?) and achieve necessary level of confidence in tests coverage
* Work on supplementary features like versionless projects/builds and version injection, better plugins paramters api and some other
Thank you
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] maximilian-novikov-db commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
maximilian-novikov-db commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r695470401
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
+
+```xml
+<?xml version="1.0" encoding="UTF-8" ?>
+<cache xmlns="org:apache:maven:cache:config:v1"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
+
+ <configuration>
+ <enabled>true</enabled>
+ <hashAlgorithm>XX</hashAlgorithm>
Review comment:
it's for xxHash which we found as a most fastest
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] qweek commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
qweek commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r695641759
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
+
+```xml
+<?xml version="1.0" encoding="UTF-8" ?>
+<cache xmlns="org:apache:maven:cache:config:v1"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
+
+ <configuration>
+ <enabled>true</enabled>
+ <hashAlgorithm>XX</hashAlgorithm>
Review comment:
Setup:
- run on Oracle JDK 1.8.0.201 on Windows and Linux
- use 100 jar files with different size from 3 KB to 90 MB, total size 474 MB
- read in single thread, calculate hash for each file and total checksum
- separate folder for each algorithm to avoid disk cache testing
- 100 rounds to minimize CPU interference from external loads
- time in seconds, candle body is 25-75 percentile, shadow is min/max values
Results:
- xxHash (5th column) shows stable time about 3-4 sec on both Windows and Linux platforms
- xxHash with Memory Mapped buffer (last column) gives another small gain on Linux, useful on CI
- SHA-256 about 4 times slower with a difference of 10 seconds even on this small test
P.S. also tested 5157 files with total size 2.6 GB, difference between xxHash and SHA-256 was about 1.5-2 min
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] maximilian-novikov-db commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
maximilian-novikov-db commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-906249184
> > > Massive! I already see room for improvement.
> > > How long did it take to go from concept to production?
> >
> >
> > 2 weeks for a PoC and a month for 1st working version in prod :)
>
> How many FTEs?
~1.5
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] olamy commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
olamy commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-904213156
> @olamy any news on this? need help?
@hboutemy my bad totally forgot this... I will have a try next week.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-914990347
@qweek had this kind of thing in mind: https://gist.github.com/rmannibucau/756156a53e792f2b68c3339db8ee97a6. Depending on bytebuffer is fine (it is in the JRE), unsafe is acceptable even if not needed and depends how you integrate with JVM - Java agreed to keep it but some version(s) can require some integration so I preferred to drop it in the project I'm using it.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] AlexanderAshitkin commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
AlexanderAshitkin commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-903311772
Hi
Thanks a lot for the feedback. The idea of this PR is not to finalize the featurem but to demonstrate curent state, and gather feedback. Realistically looking at the current state, there is an expectedly long way forward to align this feature with all the guidelines and architecure requirements. Though the feedback is reasonable and this is exactly what we are looked for, the implementation in this branch is stable and better to be kept as is. Reworks better to be done atop of that in a smaller incremental steps. To move forward following apporach appears reasonable:
1) Merge this to a feature branch as is as a baseline
2) Find way to safely enable for those who is interested under some "Experimental feature" umbrella (with minimal modifications) and merge it in an official distribution (of course safely and compliantly)
3) Gather and accumulate community feedback to produce a sort of feature roadmap. The ultimate target is integrating it to an official release with all the requirements satisified.
4) Continue work in feature branch to adress p.2 decisions and achive mutual agreement between community members
5) integrate it in an agreed/safe way and remove "Experimental flag"
Small clarifications to review comments:
1) @rmannibucau rework repositories - absolutely reasonable, should be aligned. There are some open question to discuss, like best layout for cache, associated cases - like purging cache, store cache metadata, etc. All this could evolve to repositories design discussion.
2) @rmannibucau Replace with built-in dependencies - reasonable, to be discussed. On use of jaxb - the reasone to use it was schema validation. If modello supports that - could easily be reused. ENCODING_DETECTOR - it supports wide set ofencodings beyond UTF unlike of AwareInputStreamReader. So to be discussed case by case, but in overall agree here.
4) @rfscholte on splitting PR - though technically of course possible, but decomposing seems to be a lot of work by itself. Our idea is to merge this as is to a feature branch and continue with smaller enhancements from there as necessary. They could be tageted to master or to another feature branch.
5) regarding location of the feature - it feels like it should be a seprate module at least or an extension. Happy to rework in that direction
6) @hboutemy - we will restore original formatting and similar to not add noise in review
From my perspective there are folowing implementation related streams of work
* Align implementation with built-in dependencies (like the mentioned bandwagon, modello, etc)
* Move feature to a separate module/extension safely isolated from core and find a proper way to plug it in.
* Agree proper approach to tests implementation for this feature (like introducing wiremock?) and achieve necessary level of confidence in tests coverage
Thank you
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] maximilian-novikov-db commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
maximilian-novikov-db commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-905250800
> Massive! I already see room for improvement.
> How long did it take to go from concept to production?
2 weeks for a PoC and a month for 1st working version in prod :)
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r695589792
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
+
+```xml
+<?xml version="1.0" encoding="UTF-8" ?>
+<cache xmlns="org:apache:maven:cache:config:v1"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
+
+ <configuration>
+ <enabled>true</enabled>
+ <hashAlgorithm>XX</hashAlgorithm>
Review comment:
I have noticed this on Resolver with SHA-256 and SHA-512. Horribly slow.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] qweek commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
qweek commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r695588531
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
+
+```xml
+<?xml version="1.0" encoding="UTF-8" ?>
+<cache xmlns="org:apache:maven:cache:config:v1"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
+
+ <configuration>
+ <enabled>true</enabled>
+ <hashAlgorithm>XX</hashAlgorithm>
Review comment:
No cryptographic strength is required here, in terms of quality xxHash has Score 10 in SMHasher test suite and [no reported problems](https://github.com/rurban/smhasher).
@hboutemy SHA-1, SHA-256, SHA-384, SHA-512 is also supported, just to mention that on a large project you can save several seconds just by using fast hash function.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] qweek commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
qweek commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r695588531
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
+
+```xml
+<?xml version="1.0" encoding="UTF-8" ?>
+<cache xmlns="org:apache:maven:cache:config:v1"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
+
+ <configuration>
+ <enabled>true</enabled>
+ <hashAlgorithm>XX</hashAlgorithm>
Review comment:
No cryptographic strength is required here, in terms of quality xxHash has Score 10 in SMHasher test suite and [no reported problems](https://github.com/rurban/smhasher).
@hboutemy SHA-1, SHA-256, SHA-384, SHA-512 are also supported. Just to mention that on a large project you can save several seconds by using fast hash function.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] qweek commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
qweek commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r695588531
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
+
+```xml
+<?xml version="1.0" encoding="UTF-8" ?>
+<cache xmlns="org:apache:maven:cache:config:v1"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
+
+ <configuration>
+ <enabled>true</enabled>
+ <hashAlgorithm>XX</hashAlgorithm>
Review comment:
No cryptographic strength is required here, in terms of quality xxHash has Score 10 in SMHasher test suite and [no reported problems](https://github.com/rurban/smhasher).
@hboutemy SHA-1, SHA-256, SHA-384, SHA-512 are also supported, just to mention that on a large project you can save several seconds just by using fast hash function.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] maximilian-novikov-db edited a comment on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
maximilian-novikov-db edited a comment on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-959320629
> @maximilian-novikov-db , how does this work with Idea? Does the IDE use cache? Can I import a maven submodule, or should I always import the whole monorepo in idea?
@Dunemaster
1. Yes, it uses the cache. The know limitation - if you have a code generation in your project, you will need to manually reload the project in Idea after the restoration from the cache, to sync Idea's caches. (literally click Maven/'Reload project' in menu)
2. You can do both ways, you can just import a submodule.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] hboutemy commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r693515290
##########
File path: maven-core/src/main/java/org/apache/maven/caching/CacheEventSpy.java
##########
@@ -0,0 +1,49 @@
+package org.apache.maven.caching; /* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information * regarding copyright ownership. The ASF licenses this file * to you under the Apache License, Version 2.0 (the * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * KIND, either express or implied. See the License for the * specific language governing permissions and limitations * under the License. */ /* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this
work for additional information * regarding copyright ownership. The ASF licenses this file * to you under the Apache License, Version 2.0 (the * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * KIND, either express or implied. See the License for the * specific language governing permissions and limitations * under the License. */
Review comment:
this looks like a merge typo
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] hboutemy commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r693526841
##########
File path: maven-core/src/main/java/org/apache/maven/plugin/DefaultBuildPluginManager.java
##########
@@ -134,7 +133,10 @@ public void executeMojo( MavenSession session, MojoExecution mojoExecution )
mojoExecutionListener.beforeMojoExecution( mojoExecutionEvent );
- mojo.execute();
+ if ( mojoExecution.canRun( mojo, session ) )
Review comment:
most of the modifications for this class seem to be purely formatting: restoring initial formatting would ease review
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-910129134
> > Hi Alex, Excellent work and an elaborat. Unfortunately the xxHash algorithm is not in the list https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html We must guarantee that every java would support it. T
>
> Used open-source implementation:
> [https://github.com/OpenHFT/Zero-Allocation-Hashing](https://github.com/OpenHFT/Zero-Allocation-Hashing/blob/master/src/main/java/net/openhft/hashing/XxHash.java)
FYI it is trivial to fork and takes only <150 lines and enables to drop the unsafe usage and other deps - it only depends on bytebuffer or alike - which requires some careness on recent java versions - does not dramatically change performances compared to sha algorithms.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] qweek edited a comment on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
qweek edited a comment on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-915996602
@rmannibucau I see what you mean: it will kill a few optimizations, generate more objects and probably we need to change ByteOrder (default byte buffer order is BIG_ENDIAN and algorithm order is LITTLE_ENDIAN). But you're right, it's much cleaner and simpler, thank you for sharing your vision.
[Safe version](https://github.com/qweek/Zero-Allocation-Hashing/tree/safe-version/src/main/java/net/openhft/hash) with almost 100% code coverage
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] qweek commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
qweek commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r695641759
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
+
+```xml
+<?xml version="1.0" encoding="UTF-8" ?>
+<cache xmlns="org:apache:maven:cache:config:v1"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
+
+ <configuration>
+ <enabled>true</enabled>
+ <hashAlgorithm>XX</hashAlgorithm>
Review comment:
Setup:
- run on Oracle JDK 1.8.0.201 on Windows and Linux
- use 100 jar files with different size from 3 KB to 90 MB, total size 474 MB
- read in single thread, calculate hash for each file and total checksum
- separate folder for each algorithm to avoid disk cache testing
- 100 rounds to minimize CPU interference from external loads
- time in seconds, candle body is 25-75 percentile, shadow is min/max values
Results:
- xxHash shows stable time (3-4 sec) on both Windows and Linux platforms
- xxHash with Memory Mapped buffer (XXMM) gives another small gain on Linux, useful on CI
- SHA-256 about 4 times slower with a difference of 10 seconds even on this small test
P.S. also tested 5157 files with total size 2.6 GB, difference between xxHash and SHA-256 was about 1.5-2 min
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-905254458
> > Massive! I already see room for improvement.
> > How long did it take to go from concept to production?
>
> 2 weeks for a PoC and a month for 1st working version in prod :)
How many FTEs?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] qweek edited a comment on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
qweek edited a comment on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-915996602
@rmannibucau I see what you mean: it will kill a few optimizations, generate more objects and probably we need to change ByteOrder (default byte buffer order is BIG_ENDIAN and algorithm order is LITTLE_ENDIAN). But you're right, it's much cleaner and simpler, thank you for sharing your vision.
[Safe version](https://github.com/qweek/Zero-Allocation-Hashing/tree/safe-version/src/main/java/net/openhft/hash) with almost 100% code coverage (about 7% slower, but still very fast compared to SHA)
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] hboutemy commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
hboutemy commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-903067093
@rmannibucau
> Since we got a positive vote on mvnd
@olamy any news on this? need help?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] AlexanderAshitkin edited a comment on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
AlexanderAshitkin edited a comment on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-903311772
Hi
Thanks a lot for the feedback. The idea of this PR is not to finalize the featurem but to demonstrate curent state, and gather feedback. Realistically looking at the current state, there is an expectedly long way forward to align this feature with all the guidelines and architecure requirements. Though the feedback is reasonable and this is exactly what we are looked for, the implementation in this branch is stable and better to be kept as is. Reworks better to be done atop of that in a smaller incremental steps. To move forward following apporach appears reasonable:
1) Merge this to a feature branch as is as a baseline
2) Find way to safely enable for those who is interested under some "Experimental feature" umbrella (with minimal modifications) and merge it in an official distribution (of course safely and compliantly)
3) Gather and accumulate community feedback to produce a sort of feature roadmap. The ultimate target is integrating it to an official release with all the requirements satisified.
4) Continue work in feature branch to adress p.2 decisions and achive mutual agreement between community members
5) integrate it in an agreed/safe way and remove "Experimental flag"
Small clarifications to review comments:
1) @rmannibucau rework repositories - absolutely reasonable, should be aligned. There are some open question to discuss, like best layout for cache, associated cases - like purging cache, store cache metadata, etc. All this could evolve to repositories design discussion.
2) @rmannibucau Replace with built-in dependencies - reasonable, to be discussed. On use of jaxb - the reasone to use it was schema validation. If modello supports that - could easily be reused. ENCODING_DETECTOR - it supports wide set ofencodings beyond UTF unlike of AwareInputStreamReader. So to be discussed case by case, but in overall agree here.
4) @rfscholte on splitting PR - though technically of course possible, but decomposing seems to be a lot of work by itself. Our idea is to merge this as is to a feature branch and continue with smaller enhancements from there as necessary. They could be tageted to master or to another feature branch.
5) regarding location of the feature - it feels like it should be a seprate module or an extension. Happy to rework in that direction - any guidance is very welcome
6) @hboutemy - we will restore original formatting in core and similar to not add noise in review
From my perspective there are folowing implementation related streams of work:
* Move feature to a separate module/extension safely isolated from core and find a proper way to plug it in safely.
* Align implementation with built-in dependencies (like the mentioned bandwagon, modello, etc)
* Agree proper approach to tests implementation for this feature (like introducing wiremock?) and achieve necessary level of confidence in tests coverage
Thank you
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] maximilian-novikov-db commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
maximilian-novikov-db commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-959320629
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] qweek commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
qweek commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-914958029
> FYI it is trivial to fork and takes only <150 lines and enables to drop the unsafe usage and other deps - it only depends on bytebuffer or alike - which requires some careness on recent java versions - does not dramatically change performances compared to sha algorithms.
Unfortunately it's not 150 lines of code, I [removed all unused code](https://github.com/qweek/Zero-Allocation-Hashing/tree/master/src/main/java/net/openhft) and it still depends on Unsafe and ByteBuffer
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] qweek edited a comment on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
qweek edited a comment on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-915996602
@rmannibucau I see what you mean: it will kills few optimizations, generate more objects and probably we need to use proper ByteOrder (default byte buffer order is BIG_ENDIAN and algorithm order is LITTLE_ENDIAN). But yes, it's much cleaner and simpler, thank you for sharing your vision.
[Safe version](https://github.com/qweek/Zero-Allocation-Hashing/tree/safe-version/src/main/java/net/openhft/hash) with almost 100% code coverage
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] hboutemy merged pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
hboutemy merged pull request #526:
URL: https://github.com/apache/maven/pull/526
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-902474397
Massive! I already see room for improvement.
How long did it take to go from concept to production?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] maximilian-novikov-db commented on a change in pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
maximilian-novikov-db commented on a change in pull request #526:
URL: https://github.com/apache/maven/pull/526#discussion_r695470401
##########
File path: Documentation/CACHE-HOWTO.md
##########
@@ -0,0 +1,204 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+### Overview
+
+Cache configuration provides you additional control over incremental maven behavior. Follow it step by step to
+understand how it works and figure out your optimal config
+
+### Minimal config
+
+Absolutely minimal config which enables incremental maven with local cache
+
+```xml
+<?xml version="1.0" encoding="UTF-8" ?>
+<cache xmlns="org:apache:maven:cache:config:v1"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="org:apache:maven:cache:config:v1 cache-config.xsd">
+
+ <configuration>
+ <enabled>true</enabled>
+ <hashAlgorithm>XX</hashAlgorithm>
Review comment:
it's for xxHash which we found as the most fastest
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] maximilian-novikov-db commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
maximilian-novikov-db commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-959320629
> @maximilian-novikov-db , how does this work with Idea? Does the IDE use cache? Can I import a maven submodule, or should I always import the whole monorepo in idea?
@Dunemaster Yes, it uses the cache. The know limitation - if you have a code generation in your project, you will need to manually reload the project in Idea after the restoration from the cache, to sync Idea's caches. (literally click Maven/'Reload project' in menu)
You can do both ways, you can just import a submodule.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] maximilian-novikov-db commented on pull request #526: [MNG-7129] Incremental build and shared cache feature
Posted by GitBox <gi...@apache.org>.
maximilian-novikov-db commented on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-959320629
> @maximilian-novikov-db , how does this work with Idea? Does the IDE use cache? Can I import a maven submodule, or should I always import the whole monorepo in idea?
@Dunemaster Yes, it uses the cache. The know limitation - if you have a code generation in your project, you will need to manually reload the project in Idea after the restoration from the cache, to sync Idea's caches. (literally click Maven/'Reload project' in menu)
You can do both ways, you can just import a submodule.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org