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

Michael McCandless commented on LUCENE-1383:
--------------------------------------------

bq. The current patch solves the problem suitably for Lucene, at the expense of 
some performance degradation.

The performance degradation should be negligible in the case of long-lived 
threads.  No synchronized code was added in the get() path.

bq. It is also not valid, since you need to clear all values across all 
threads, remove() only clears the entry for the calling thread.

This best practice does work correctly: you're supposed to call remove() from 
the very thread that had inserted something into the ThreadLocal.

Besides the 1.5 issue, this is also difficult for Lucene because we don't have 
a clean point to "know" when the thread has finished interacting with Lucene.  
A thread comes out of the pool, runs a search, gathers docIDs from in a 
collector, returns from the search, one by one looks up stored docs / term 
vectors for these docIDs, maybe does secondary search up front to build up 
filters, etc., finishes rendering the result and returns to the pool.  Unless 
we create a new method "detachThread(...)" somewhere in Lucene, there is no 
natural point now to do the remove().  And I don't like creating such a method 
because nobody will ever know they need to call it in their App server until 
they start hitting cryptic OOMs.

> Work around ThreadLocal's "leak"
> --------------------------------
>
>                 Key: LUCENE-1383
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1383
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Index
>    Affects Versions: 1.9, 2.0.0, 2.1, 2.2, 2.3, 2.3.1, 2.3.2
>            Reporter: Michael McCandless
>            Assignee: Michael McCandless
>             Fix For: 2.4
>
>         Attachments: LUCENE-1383.patch, ScreenHunter_01 Sep. 13 08.40.jpg, 
> ScreenHunter_02 Sep. 13 08.42.jpg, ScreenHunter_03 Sep. 13 08.43.jpg, 
> ScreenHunter_07 Sep. 13 19.13.jpg
>
>
> Java's ThreadLocal is dangerous to use because it is able to take a
> surprisingly very long time to release references to the values you
> store in it.  Even when a ThreadLocal instance itself is GC'd, hard
> references to the values you had stored in it are easily kept for
> quite some time later.
> While this is not technically a "memory leak", because eventually
> (when the underlying Map that stores the values cleans up its "stale"
> references) the hard reference will be cleared, and GC can proceed,
> its end behavior is not different from a memory leak in that under the
> right situation you can easily tie up far more memory than you'd
> expect, and then hit unexpected OOM error despite allocating an
> extremely large heap to your JVM.
> Lucene users have hit this many times.  Here's the most recent thread:
>   
> http://mail-archives.apache.org/mod_mbox/lucene-java-dev/200809.mbox/%3C6e3ae6310809091157j7a9fe46bxcc31f6e63305fcdc%40mail.gmail.com%3E
> And here's another:
>   
> http://mail-archives.apache.org/mod_mbox/lucene-java-dev/200807.mbox/%3CF5FC94B2-E5C7-40C0-8B73-E12245B91CEE%40mikemccandless.com%3E
> And then there's LUCENE-436 and LUCENE-529 at least.
> A google search for "ThreadLocal leak" yields many compelling hits.
> Sun does this for performance reasons, but I think it's a terrible
> trap and we should work around it with Lucene.

-- 
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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to