[
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: [email protected]
For additional commands, e-mail: [email protected]
