[
https://issues.apache.org/jira/browse/LUCENE-1749?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12739092#action_12739092
]
Hoss Man commented on LUCENE-1749:
----------------------------------
General Comments on mark's latest patch...
* the changes that i understand all seem good ... some of the details in
reader/searcher/query internals elude me but it sounds Yonik & McCandless have
their eyes on them so i trust the three of you have it covered.
* we still need to fill in some empty/sparse javadocs, but that can be done
after an initial commit.
* is it a bug that AverageGuessMemoryModel.getSize() will NPE on a non
primitive class ... or should/will the docs for that API say it only works on
primitives?
Big Questions I Still Have....
* does anyone have any reservations about the new APIs introduced?
* FieldCache.CreationPlaceholder (promoted from FieldCacheImpl)
* FieldCache.CacheEntry
* FieldCache.getCacheEntries()
* FieldCache.purgeAllCaches()
* FieldCacheSanityChecker
* RamUsageEstimator
* does anyone have any reservations about the refactoring done in
FieldCacheImpl to make this new API possible? (ie: did i break the thread
safety in a way i'm not noticing?)
* is the FieldCacheImpl.Entry.type (the "SortField" int type) still needed by
FieldCacheImpl.Entry? ... nothing seems to use it so it would be nice to
eliminate it and simplify the CacheEntry API as well. (i suspect it got
refactored into obsolescence when the Sorting got moved into the subreaders)
* The sanity checking ignores CreationPlaceholder -- largely because of the way
the numeric caches first try one parser, and then if they get an NFE try a
different parser -- but this leaves the CreationPlaceholder in the cache. It's
not a big object, so i assume it was implemented this way on purpose and the
sanity checker is doing the correct thing by ignoring it, but i wanted to make
sure people are aware/ok with this behavior.
Lastly: This patch feels unnecessarily large at this point. Several of the
bugs/improvements we've uncovered here don't seem like belong in this patch,
and should be tracked in separate Jira issues, which can be committed
independently and enumerated in CHANGES.txt....
* new ReaderUtil and the usage in DirectoryReader, MultiReader, MultiSearcher
& IndexSearcher
* explain subreader bug fixes in ConstantScoreQuery, QueryWeight,
ValueSourceQuery, CustomScoreQuery, etc...
...i think this issue (and this patch) should be reduced to just the new sanity
checkig API, and *tests* that have been changed to be more sane (where the
underlying code was already fine)
Mark: would you mind splitting up the latest patch you have (you mentioned some
additional minor tweaks) and opening new issues for these peripheral changes
and then attaching back what's left for this patch. Then I'll take the conch
back and work on the missing javadocs.
(I'll happily commit once i get at least one thumbs up from someone on the "Big
Questions" above ... we can always tweak the javadocs further in subsequent
commits)
> FieldCache introspection API
> ----------------------------
>
> Key: LUCENE-1749
> URL: https://issues.apache.org/jira/browse/LUCENE-1749
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Search
> Reporter: Hoss Man
> Priority: Minor
> Fix For: 2.9
>
> Attachments: fieldcache-introspection.patch,
> LUCENE-1749-hossfork.patch, LUCENE-1749.patch, LUCENE-1749.patch,
> LUCENE-1749.patch, LUCENE-1749.patch, LUCENE-1749.patch, LUCENE-1749.patch,
> LUCENE-1749.patch, LUCENE-1749.patch, LUCENE-1749.patch, LUCENE-1749.patch
>
>
> FieldCache should expose an Expert level API for runtime introspection of the
> FieldCache to provide info about what is in the FieldCache at any given
> moment. We should also provide utility methods for sanity checking that the
> FieldCache doesn't contain anything "odd"...
> * entries for the same reader/field with different types/parsers
> * entries for the same field/type/parser in a reader and it's subreader(s)
> * etc...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: java-dev-h...@lucene.apache.org
