[jira] [Commented] (LUCENE-6365) Optimized iteration of finite strings

["Dawid Weiss (JIRA)" <ji...@apache.org>]

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

Dawid Weiss commented on LUCENE-6365:
-------------------------------------

I looked at the patch (briefly, without delving into all the details -- I'm on vacation :). 

I like the idea overall. The only thing that bugs me is the explicitness of FiniteStringsIterator. I would rather have something like:
{code}
Iterator<IntsRef> Operations.iterateFiniteStrings(...)
{code}
so that we can leave FiniteStringsIterator as a package-private implementation and not proliferate it around. If Iterator interface gives you a headache (which it can) then I'd leave FiniteStringsIterator as an explicit return type but leave the factory method inside Operations.

The assignments inside the conditional part of {{for}} loops are very likely to drive somebody crazy sooner or later.

Again, I only looked at the patch, I don't even have a proper environment with me to check on all the details.

> Optimized iteration of finite strings
> -------------------------------------
>
>                 Key: LUCENE-6365
>                 URL: https://issues.apache.org/jira/browse/LUCENE-6365
>             Project: Lucene - Core
>          Issue Type: Improvement
>          Components: core/other
>    Affects Versions: 5.0
>            Reporter: Markus Heiden
>            Priority: Minor
>              Labels: patch, performance
>         Attachments: FiniteStringsIterator.patch
>
>
> Replaced Operations.getFiniteStrings() by an optimized FiniteStringIterator.
> Benefits:
> Avoid huge hash set of finite strings.
> Avoid massive object/array creation during processing.
> "Downside":
> Iteration order changed, so when iterating with a limit, the result may differ slightly. Old: emit current node, if accept / recurse. New: recurse / emit current node, if accept.
> The old method Operations.getFiniteStrings() still exists, because it eases the tests. It is now implemented by use of the new FiniteStringIterator.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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