Note to QA - Best done via code review

@scott,

My team (and myself) are currently on bugs and would love to fix this. When I found this in 2007 I was less "experienced" in changing JIRA. These days I would rip into it in an jiffy!

Attached is a patch I was working on at one stage. It needs a bit more testing, but it removes a lot of the problem code. Javadoc will need to be updated if incorporated.

Mark,

Can you cast you highly tuned performance mind towards this one and gives us an opinion. This has been around for ages and "curiosity" of old code however the people from Yandex are reporting it as a significant performance blocker for them.

They have helped identify performace bugs for us in the past with some success.

    /**
     * Put an object into the cache.
     */
    Object store(IndexReader reader, String field, int type, Object value)
    {
        Entry entry = new Entry(field, type);
        synchronized (this)
        {
            HashMap readerCache = (HashMap) cache.get(reader);
            if (readerCache == null)
            {
                readerCache = new HashMap();
                // don't cache these objects - this seems to be a memory leak, and results in
                // no increase in performance - JRA-10111
                //cache.put(reader, readerCache);
            }
            return readerCache.put(entry, value);
        }
    }

Commenting out this block without removing whole class was big mistake!!! Whole class contains no caching and only synbchronized blocks which always returns nulls (look in lookup methods)?

This is major bottleneck under high load and directly relates to JRA-21557.

There has been no movement per se because this is is not actually a performance problem but a code refactoring problem. My understanding is that there is redundant but not used cache code in the code that should be removed but is currently harmless and not used.

If you know of a reason why this is not true and it does in fact have a performance problem, please let us know..

ping
Any progress on the issue

ping!

Hey - am fine to remove it. You may just want to check the Lucene source code before you do.

The Lucene in Action books states (on page 199) "The sorting infrastructure within Lucene caches (based on a key combining the hashcode of the IndexReader, the field name and the custom sort object) the results of SortComparatorSource.newComparator"

So I would hazard a guess that Lucene is already caching this information for us any way and hence a secondary cache on top is not needed. This is probably why it never had a performance problem once the old caching "put" code was commented out.