[jira] [Comment Edited] (LUCENE-4946) Refactor SorterTemplate

["Uwe Schindler (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LUCENE-4946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13648001#comment-13648001 ] 

Uwe Schindler edited comment on LUCENE-4946 at 5/2/13 11:04 PM:
----------------------------------------------------------------

Hi Adrien,
thansk for the refactoring. The history of the SorterTemplate class going back to CGLIB is long and this is a really good idea. Its also useful for other projects, so its maybe a good idea to make a Apache Commons projects out of it :-)

I scanned the patch, looks good. The from...to semantics are better now for the user. I think the original implementation used inclusive end because most implementations on the web were based on this. For me it always looked wrong, but I did not want to change it.

I found some code duplication: To me it looks like ArrayUtil has a private re-implementation of ArrayIntroSorter which is a top-level class in oal.util. Could ArrayUtil not simply use that public impl instead? I know there are 2 implementations with Comparators and without comparators, just an idea! Maybe add a static final singleton NaturalComparator<T extends Comparable<? super T>> that calls compareTo, so we dont need 2 implementations.

I also like that you used timsort at places were the lists are already sorted in the common case (like Automatons).
                
      was (Author: thetaphi):
    Hi Adrien,
thansk for the refactoring. The history of the SorterTemplate class going back to CGLIB is long and this is a really good idea. Its also useful for other projects, so its maybe a good idea to make a Apache Commons projects out of it :-)

I scanned the patch, looks good. The from...to semantics are better now for the user. I think the original implementation used inclusive end because most implementations on the web were based on this. For me it always looked wrong, but I did not want to change it.

I found some code duplication: To me it looks like ArrayUtil has a private re-implementation of ArrayIntroSorter which is a top-level class in oal.util. Could ArrayUtil not simply use that public impl instead? I know there are 2 implementations with Comparators and without comparators, just an idea! Maybe add a static final singleton NaturalComparator<Comparable<?>> that calls compareTo, so we dont need 2 implementations.

I also like that you used timsort at places were the lists are already sorted in the common case (like Automatons).
                  
> Refactor SorterTemplate
> -----------------------
>
>                 Key: LUCENE-4946
>                 URL: https://issues.apache.org/jira/browse/LUCENE-4946
>             Project: Lucene - Core
>          Issue Type: Improvement
>            Reporter: Adrien Grand
>            Assignee: Adrien Grand
>            Priority: Trivial
>         Attachments: LUCENE-4946.patch, LUCENE-4946.patch
>
>
> When working on TimSort (LUCENE-4839), I was a little frustrated of not being able to add galloping support because it would have required to add new primitive operations in addition to compare and swap.
> I started working on a prototype that uses inheritance to allow some sorting algorithms to rely on additional primitive operations. You can have a look at https://github.com/jpountz/sorts/tree/master/src/java/net/jpountz/sorts (but beware it is a prototype and still misses proper documentation and good tests).
> I think it would offer several advantages:
>  - no more need to implement setPivot and comparePivot when using in-place merge sort or insertion sort,
>  - the ability to use faster stable sorting algorithms at the cost of some memory overhead (our in-place merge sort is very slow),
>  - the ability to implement properly algorithms that are useful on specific datasets but require different primitive operations (such as TimSort for partially-sorted data).
> If you are interested in comparing these implementations with Arrays.sort, there is a Benchmark class in src/examples.
> What do you think?

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org