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 2020/10/11 21:37:55 UTC

[GitHub] [maven-resolver] michaelboyles opened a new pull request #76: [MRESOLVER-141] - Review index-based access to collections

michaelboyles opened a new pull request #76:
URL: https://github.com/apache/maven-resolver/pull/76


   https://issues.apache.org/jira/browse/MRESOLVER-141
   
   Most of the usages I didn't change were things where they were just explicitly accessing the last element, which is constant-time for LinkedList and RandomAccess lists 


----------------------------------------------------------------
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

[GitHub] [maven-resolver] michaelboyles commented on pull request #76: [MRESOLVER-141] - Review index-based access to collections

Posted by GitBox <gi...@apache.org>.
michaelboyles commented on pull request #76:
URL: https://github.com/apache/maven-resolver/pull/76#issuecomment-710431377


   I didn't miss them. I decided not to change them.
   
   **DefaultDependencyCollector partially (line 758)**
   
   It's a NodeStack. A custom class which does not support iterators. It only has size() and get(). I could add an iterator method, but its backed by an array, so already has constant time random access. Wouldn't give any performance benefit
   
   **PathRecordingDependencyVisitor**
   
   Again, custom class backed by an array, this time Stack.
   
   Could change to this 
   
   ```
   List<DependencyNode> path = new ArrayList<>( this.parents );
   Collections.reverse( path );
   paths.add( path );
   ```
   
   which I think is the same result but more expressive. Probably actually slower because of the copy-then-reverse. Or at least, no performance benefit
   
   ***GenericVersion***
   
   As far as I can tell, the index is necessary


----------------------------------------------------------------
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

[GitHub] [maven-resolver] michael-o commented on pull request #76: [MRESOLVER-141] - Review index-based access to collections

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #76:
URL: https://github.com/apache/maven-resolver/pull/76#issuecomment-706775666


   Wow, really really nice! Thank you! I will happily review this 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.

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

[GitHub] [maven-resolver] michael-o commented on pull request #76: [MRESOLVER-141] - Review index-based access to collections

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #76:
URL: https://github.com/apache/maven-resolver/pull/76#issuecomment-710692030


   Agreed.


----------------------------------------------------------------
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

[GitHub] [maven-resolver] michaelboyles commented on pull request #76: [MRESOLVER-141] - Review index-based access to collections

Posted by GitBox <gi...@apache.org>.
michaelboyles commented on pull request #76:
URL: https://github.com/apache/maven-resolver/pull/76#issuecomment-710697704


   Thanks for reviewing


----------------------------------------------------------------
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

[GitHub] [maven-resolver] asfgit closed pull request #76: [MRESOLVER-141] - Review index-based access to collections

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #76:
URL: https://github.com/apache/maven-resolver/pull/76


   


----------------------------------------------------------------
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