Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MASSEMBLY-945 - speed improvements #38

Merged
merged 1 commit into from Dec 11, 2020
Merged

MASSEMBLY-945 - speed improvements #38

merged 1 commit into from Dec 11, 2020

Conversation

gnodet
Copy link
Contributor

@gnodet gnodet commented Nov 18, 2020

This PR only makes sense with
codehaus-plexus/plexus-utils#106
codehaus-plexus/plexus-io#33
codehaus-plexus/plexus-archiver#157
apache/maven-common-artifact-filters#15

... but if I add the snapshot upgrades, the checks won't pass anymore ;-)

@elharo
Copy link
Contributor

elharo commented Dec 7, 2020

have you measured the improvements? if so, what are the numbers?

@bmarwell
Copy link

bmarwell commented Dec 7, 2020

have you measured the improvements? if so, what are the numbers?

Null-length arrays are known to be faster since at least 2013 iirc.

In my opinion measurements are not needed here because of this.

@elharo
Copy link
Contributor

elharo commented Dec 7, 2020

Far more often than not, microoptimizations like this result in no measurable impact. All claims of performance improvements need numbers.

@bmarwell
Copy link

Far more often than not, microoptimizations like this result in no measurable impact. All claims of performance improvements need numbers.

codehaus-plexus/plexus-io#33 (comment)

@bmarwell bmarwell requested a review from olamy December 10, 2020 12:53
@gnodet
Copy link
Contributor Author

gnodet commented Dec 10, 2020

The speed gain may be negligible for this PR, it does not matter much.
However, I'd like the other PR referenced above to get merged, so that we can upgrade to those new versions to have a non negligible performance gain.

@olamy olamy merged commit 9cbdbc4 into apache:master Dec 11, 2020
@eolivelli
Copy link
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants