[
https://issues.apache.org/jira/browse/LUCENE-5316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13830874#comment-13830874
]
Shai Erera commented on LUCENE-5316:
------------------------------------
Thanks Mike. Looks like for the usual case, these changes do improve
performance slightly (up to 7%). What's up w/ NO_PARENTS though? The code still
iterates on an int[], with this patch it even iterates on an int[] w/o any
skips, yet we see 70% slowdown!? And more so, the new structure should consume
much less RAM than PTA, since we don't keep an int (-1) for the majority of the
categories that are leaf nodes - which means more of these ints can fit into
the CPU cache than in PTA? I don't think that Hashing into the map is
significant since it happens once for the dim, but then the code iterates on
the children[] directly?
One thing I note in the patch is that IntRollupFacetsAggregator now seems to do
two 'ifs' per child: (1) the for-loop looping on all children of X, and (2) for
each child, check if it has children before recursing. I think that has two
effects, not present in trunk:
* An extra 'if' - though it's arguable if that 'if' isn't in fact saving much
more than on trunk, where we recurse in most cases for nothing. I believe
recursion will be more expensive?
* A call to TR.getChildren(), which in turn makes all these calls:
** getChildrenMap()
*** ensureOpen()
*** if (children == null)
** map.get(ordinal)
** if (kids == null)
So actually, looks like w/ this patch (since we don't have a single array), we
lose in the rollup case (unlike what I wrote above), because for *each* child
we make all these calls - none of them are present in trunk. I believe they add
up .. accessing volatile members, hashing every one of the 2.5M categories ...
I believe that's the slowdown that we see. What do you think?
So I see two ways to proceed:
# Close the issue as "Won't Fix" since the gains aren't high, yet the potential
slowdown is substantial. We can reopen in the future if anything changes
# Proceed w/ the map approach since it improves in the regular case, reduces
RAM consumption and makes the API cleaner and more intuitive. Also, w/ Mike's
changes on LUCENE-5539, we won't end up in a NO_PARENTS case for all flat
dimensions...
#* Well actually we could wait with this issue until LUCENE-5539 is resolved
and then re-evaluate the changes. I still believe that for the hierarchical
dimensions where we index leafs only, this approach will lose (because of all
the extra calls I made above). But then someone will always be able to tell
FacetsConfig that this is a multi-valued dim to enforce the ALL_BUT_DIM
encoding, and gain speedups back ...
What do you think?
> Taxonomy tree traversing improvement
> ------------------------------------
>
> Key: LUCENE-5316
> URL: https://issues.apache.org/jira/browse/LUCENE-5316
> Project: Lucene - Core
> Issue Type: Improvement
> Components: modules/facet
> Reporter: Gilad Barkai
> Priority: Minor
> Attachments: LUCENE-5316.patch, LUCENE-5316.patch, LUCENE-5316.patch,
> LUCENE-5316.patch, LUCENE-5316.patch
>
>
> The taxonomy traversing is done today utilizing the
> {{ParallelTaxonomyArrays}}. In particular, two taxonomy-size {{int}} arrays
> which hold for each ordinal it's (array #1) youngest child and (array #2)
> older sibling.
> This is a compact way of holding the tree information in memory, but it's not
> perfect:
> * Large (8 bytes per ordinal in memory)
> * Exposes internal implementation
> * Utilizing these arrays for tree traversing is not straight forward
> * Lose reference locality while traversing (the array is accessed in
> increasing only entries, but they may be distant from one another)
> * In NRT, a reopen is always (not worst case) done at O(Taxonomy-size)
> This issue is about making the traversing more easy, the code more readable,
> and open it for future improvements (i.e memory footprint and NRT cost) -
> without changing any of the internals.
> A later issue(s?) could be opened to address the gaps once this one is done.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
