From issues-return-151468-archive-asf-public=cust-asf.ponee.io@maven.apache.org Mon Jul 1 18:50:26 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id B91A8180670 for ; Mon, 1 Jul 2019 20:50:25 +0200 (CEST) Received: (qmail 56809 invoked by uid 500); 1 Jul 2019 18:50:25 -0000 Mailing-List: contact issues-help@maven.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@maven.apache.org Delivered-To: mailing list issues@maven.apache.org Received: (qmail 56797 invoked by uid 99); 1 Jul 2019 18:50:25 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 01 Jul 2019 18:50:25 +0000 From: GitBox To: issues@maven.apache.org Subject: [GitHub] [maven] gnodet commented on a change in pull request #262: Improve speed in collection merging Message-ID: <156200702463.6669.9192787648287801911.gitbox@gitbox.apache.org> Date: Mon, 01 Jul 2019 18:50:24 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit gnodet commented on a change in pull request #262: Improve speed in collection merging URL: https://github.com/apache/maven/pull/262#discussion_r299171614 ########## File path: maven-model/src/main/java/org/apache/maven/model/merge/ModelMerger.java ########## @@ -3018,4 +2618,298 @@ protected Object getExclusionKey( Exclusion exclusion ) return exclusion; } + interface KeyComputer + { + Object key( T t ); + } + + class DependencyKeyComputer implements KeyComputer + { + @Override + public Object key( Dependency dependency ) + { + return getDependencyKey( dependency ); + } + } + + class LicenseKeyComputer implements KeyComputer + { + @Override + public Object key( License license ) + { + return getLicenseKey( license ); + } + } + + class MailingListKeyComputer implements KeyComputer + { + @Override + public Object key( MailingList mailingList ) + { + return getMailingListKey( mailingList ); + } + } + + class DeveloperKeyComputer implements KeyComputer + { + @Override + public Object key( Developer developer ) + { + return getDeveloperKey( developer ); + } + } + + class ContributorKeyComputer implements KeyComputer + { + @Override + public Object key( Contributor contributor ) + { + return getContributorKey( contributor ); + } + } + + class ProfileKeyComputer implements KeyComputer + { + @Override + public Object key( Profile profile ) + { + return getProfileKey( profile ); + } + } + + class RepositoryKeyComputer implements KeyComputer + { + @Override + public Object key( Repository repository ) + { + return getRepositoryKey( repository ); + } + } + + class ReportPluginKeyComputer implements KeyComputer + { + @Override + public Object key( ReportPlugin plugin ) + { + return getReportPluginKey( plugin ); + } + } + + class PluginKeyComputer implements KeyComputer + { + @Override + public Object key( Plugin plugin ) + { + return getPluginKey( plugin ); + } + } + + class ReportSetKeyComputer implements KeyComputer + { + @Override + public Object key( ReportSet reportSet ) + { + return getReportSetKey( reportSet ); + } + } + + class NotifierKeyComputer implements KeyComputer + { + @Override + public Object key( Notifier notifier ) + { + return getNotifierKey( notifier ); + } + } + + class ExtensionKeyComputer implements KeyComputer + { + @Override + public Object key( Extension extension ) + { + return getExtensionKey( extension ); + } + } + + class ResourceKeyComputer implements KeyComputer + { + @Override + public Object key( Resource resource ) + { + return getResourceKey( resource ); + } + } + + class ExecutionKeyComputer implements KeyComputer + { + @Override + public Object key( PluginExecution pluginExecution ) + { + return getPluginExecutionKey( pluginExecution ); + } + } + + class ExclusionKeyComputer implements KeyComputer + { + @Override + public Object key( Exclusion exclusion ) + { + return getExclusionKey( exclusion ); + } + } + + static List merge( List tgt, List src, boolean sourceDominant, KeyComputer computer ) + { + if ( src.isEmpty() ) + { + return tgt; + } + + MergingList list; + if ( tgt instanceof MergingList ) + { + list = (MergingList) tgt; + } + else + { + list = new MergingList<>( computer, src.size() + tgt.size() ); + list.mergeAll( tgt, true ); + } + + list.mergeAll( src, sourceDominant ); + return list; + } + + /** + * Merging list + * @param + */ + public static class MergingList extends AbstractList Review comment: @Tibor17 My goal here is just speed performances, nothing else. I don't want to refactor anything and my main goal is to keep 100% compatibility. That's the reasons for the sometime weird choices that were made in this PR. I don't think the `List` interface specific methods such as `listIterator`, `add(Object, int)` or such are used at all, but given the aim of 100% compatibility, I decided to make them available in the resulting collection. So while your suggestions may be totally appropriate, this would not be compatible, so this is definitely a separate discussion, a separate jira issue, and a separate target release (i'm aiming for 100% compatibility to have this included in a 3.6.2 if possible). The `c instanceof MergingList` has been added to be able to reuse this dual data structure and avoid switching between the map and list quite a few times. I'll add a commit to make the `MergingList` private and the `KeyComputer` protected (so that they can be reused in the later derived classes). Again the KeyComputer interface and implementations should completely go away once Maven requires JDK8, but again, that's a different target/discussion. @eolivelli you're right, the refactoring is mainly a side effect (though a good one), but definitely my main goal was speed improvement. The lists can't be made immutable. The reason is that some plugins do modify them and manipulate the project directly, for example when dynamically adding `Resource` after generation. So that's definitely not possible. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: users@infra.apache.org With regards, Apache Git Services