lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David Smiley (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LUCENE-5745) Refactoring AbstractVisitingPrefixTreeFilter code using cellIterator.
Date Sun, 15 Jun 2014 05:40:01 GMT

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

David Smiley commented on LUCENE-5745:
--------------------------------------

I took a look at this today.  Your code was good in that it was how I thought it would be.
 When I debugged why it didn't work, I made a couple realizations:
* cell.remove() wasn't implemented on the CellIterator implementation returned from LegacyPrefixTree
when the query shape is a Point (FilteredCellIterator). I had LPT augment it via an anonymous
class.
* findSubCellsToVisit() wasn't quite right; I changed the abstraction so up-front IsWithin
could customize the shape that is used for getTreeCellIterator(shape).
* It may not have mattered to get the tests to pass but I realized cell.remove() needed to
be invoked in a few other cases as well.

I also made some basic refactorings like inlining addIntersectingChildren().  The lines of
code in AVPTF went from 390 to 277 (most of which you already did), which is pretty darned
sweet.  This refactoring also means a reduction in allocated objects -- AVPT introduces no
new allocation on a per-cell or per-term basis.

BUT, I couldn't get the IsWithin predicate to work.  I debugged further and found out why.
 Essentially, IsWithin wants to have more control over descending down the SPT, even when
the current cell is a leaf.  But TreeCellIterator can't be told to go down, only to not go
down.  And it won't go down when the current cell is a leaf.  I think I need to monkey with
the APIs a bit to make this work.  I'm currently leaning towards TreeCellIterator being an
interface (current code moving to an "Impl" suffix) that gives the caller control over descending,
independent of wether the cell is a leaf.  Then, SPT.getTreeCellIterator would return an actual
TreeCellIterator.

> Refactoring AbstractVisitingPrefixTreeFilter code using cellIterator.
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-5745
>                 URL: https://issues.apache.org/jira/browse/LUCENE-5745
>             Project: Lucene - Core
>          Issue Type: Improvement
>          Components: modules/spatial
>            Reporter: Varun  V Shenoy
>            Priority: Minor
>             Fix For: 5.0
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> The AbstractVisitingPrefixTreeFilter (used by RPT's Intersects, Within, Disjoint) really
should be refactored to use the new CellIterator API as it will reduce the amount of code
and should make the code easier to follow since it would be based on a well-known design-pattern
(an iterator). It currently uses a VNode and VNode Iterator.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

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


Mime
View raw message