You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Yonik Seeley (JIRA)" <ji...@apache.org> on 2006/07/08 03:00:30 UTC

[jira] Commented: (LUCENE-623) RAMDirectory.close() should have a comment about not releasing any resources

    [ http://issues.apache.org/jira/browse/LUCENE-623?page=comments#action_12419805 ] 

Yonik Seeley commented on LUCENE-623:
-------------------------------------

Just out of curiousity, does Lucene hold onto a RAMDirectory instance somewhere after it has called close on it?  If so, that would have been the ideal place to fix it any kind of "leak".

This patch, while not doing much harm, is not really "best practice" in Java and the existing RAMDirectory close() was correct.
Am I missing something?

> RAMDirectory.close() should have a comment about not releasing any resources
> ----------------------------------------------------------------------------
>
>          Key: LUCENE-623
>          URL: http://issues.apache.org/jira/browse/LUCENE-623
>      Project: Lucene - Java
>         Type: Improvement

>   Components: Store
>     Versions: 1.9
>     Reporter: Heng Mei
>     Assignee: Hoss Man
>     Priority: Minor
>  Attachments: ramdirectory.diff
>
> I wrongly assumed that calling RAMDirectory.close() would free up the memory occupied by the RAMDirectory.
> It might be helpful to add a javadoc comment that warns users that RAMDirectory.close() is a no-op, since it might be a common assumption that close() would release resources.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


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


Re: [jira] Commented: (LUCENE-623) RAMDirectory.close() should have a comment about not releasing any resources

Posted by robert engels <re...@ix.netcom.com>.
I don't think there is any leak at all. the problem was the caller  
was maintaining a reference to the reader after close was called  
(which prevents usage anyway), and wanted to free some of the  
underlying memory - kind of pointless, he should just release the ref  
to the IndexReader (since it is unusable anyway).

The patch was not needed.

On Jul 7, 2006, at 8:00 PM, Yonik Seeley (JIRA) wrote:

>     [ http://issues.apache.org/jira/browse/LUCENE-623? 
> page=comments#action_12419805 ]
>
> Yonik Seeley commented on LUCENE-623:
> -------------------------------------
>
> Just out of curiousity, does Lucene hold onto a RAMDirectory  
> instance somewhere after it has called close on it?  If so, that  
> would have been the ideal place to fix it any kind of "leak".
>
> This patch, while not doing much harm, is not really "best  
> practice" in Java and the existing RAMDirectory close() was correct.
> Am I missing something?
>
>> RAMDirectory.close() should have a comment about not releasing any  
>> resources
>> --------------------------------------------------------------------- 
>> -------
>>
>>          Key: LUCENE-623
>>          URL: http://issues.apache.org/jira/browse/LUCENE-623
>>      Project: Lucene - Java
>>         Type: Improvement
>
>>   Components: Store
>>     Versions: 1.9
>>     Reporter: Heng Mei
>>     Assignee: Hoss Man
>>     Priority: Minor
>>  Attachments: ramdirectory.diff
>>
>> I wrongly assumed that calling RAMDirectory.close() would free up  
>> the memory occupied by the RAMDirectory.
>> It might be helpful to add a javadoc comment that warns users that  
>> RAMDirectory.close() is a no-op, since it might be a common  
>> assumption that close() would release resources.
>
> -- 
> This message is automatically generated by JIRA.
> -
> If you think it was sent incorrectly contact one of the  
> administrators:
>    http://issues.apache.org/jira/secure/Administrators.jspa
> -
> For more information on JIRA, see:
>    http://www.atlassian.com/software/jira
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-dev-help@lucene.apache.org
>


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


Re: [jira] Commented: (LUCENE-623) RAMDirectory.close() should have a comment about not releasing any resources

Posted by robert engels <re...@ix.netcom.com>.
I think the only time setting a object reference to null is during a  
"close" type operation is if the object is in some way shared (or  
could be), and the top-level object can be reopened or used again.  
Since no operations are valid on an IndexReader after close, the  
releasing of private object references seems to just be "noise".

On Jul 9, 2006, at 1:58 AM, Chris Hostetter wrote:

>
> : Just out of curiousity, does Lucene hold onto a RAMDirectory  
> instance
> : somewhere after it has called close on it?  If so, that would  
> have been
> : the ideal place to fix it any kind of "leak".
>
> Not that I'm aware of.  In my opinion there was no leak, i was just  
> trying
> to make RAMDirectory.close() do it's best to release the resources  
> it was
> holding on to.
>
> : This patch, while not doing much harm, is not really "best  
> practice" in
> : Java and the existing RAMDirectory close() was correct.
>
> Why is this not a best practice?  (or do you mean that in a "not
> particularly good" sense instead of "bad" sense?)
>
>
>
> -Hoss
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-dev-help@lucene.apache.org
>


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


Re: [jira] Commented: (LUCENE-623) RAMDirectory.close() should have a comment about not releasing any resources

Posted by Yonik Seeley <ys...@gmail.com>.
On 7/10/06, Chris Hostetter <ho...@fucit.org> wrote:
> [performance stuff]
> Is this serious enough to revert?

Definitely not serious at all.  It won't even be measurable.
Uncovering bugs that may only have manifested in FSDirectory is more
than enough reason to not revert.

-Yonik
http://incubator.apache.org/solr Solr, the open-source Lucene search server

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


Re: [jira] Commented: (LUCENE-623) RAMDirectory.close() should have a comment about not releasing any resources

Posted by Chris Hostetter <ho...@fucit.org>.
: I should metion that there is an upside to the patch.... it can
: uncover bugs by detecting access after a close().  Before, this would
: have worked with a RAMDirectory, but failed with a FSDirectory.

yeah, seeing that test failure when i made the change locally is what sold
me on commiting it -- that and it seemed to demonstrate the principle of
least suprise, the dir.close() releases the resources it has -- which may
not truely do anything at run time, but someone reading the code won't
scratch there head.

i didn't realize that it can potentially hurt performance as yonik
mentioned (frankly: i still don't understand how that can be the case, but
when yonik makes a statement about performance: i believe him.)


Is this serious enough to revert?


-Hoss


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


Re: [jira] Commented: (LUCENE-623) RAMDirectory.close() should have a comment about not releasing any resources

Posted by Yonik Seeley <ys...@gmail.com>.
I should metion that there is an upside to the patch.... it can
uncover bugs by detecting access after a close().  Before, this would
have worked with a RAMDirectory, but failed with a FSDirectory.

-Yonik
http://incubator.apache.org/solr Solr, the open-source Lucene search server

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


Re: [jira] Commented: (LUCENE-623) RAMDirectory.close() should have a comment about not releasing any resources

Posted by Yonik Seeley <ys...@gmail.com>.
On 7/9/06, Chris Hostetter <ho...@fucit.org> wrote:
> : This patch, while not doing much harm, is not really "best practice" in
> : Java and the existing RAMDirectory close() was correct.
>
> Why is this not a best practice?  (or do you mean that in a "not
> particularly good" sense instead of "bad" sense?)

Right... some people used to null out references with the idea that it
would help out GC.  With modern VMs, it's normally a net performance
loss.  If the object you are releasing contains other references, one
should not normally null those references.

It only makes sense to null references when the lifetime of the parent
is greater.

-Yonik
http://incubator.apache.org/solr Solr, the open-source Lucene search server

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


Re: [jira] Commented: (LUCENE-623) RAMDirectory.close() should have a comment about not releasing any resources

Posted by Chris Hostetter <ho...@fucit.org>.
: Just out of curiousity, does Lucene hold onto a RAMDirectory instance
: somewhere after it has called close on it?  If so, that would have been
: the ideal place to fix it any kind of "leak".

Not that I'm aware of.  In my opinion there was no leak, i was just trying
to make RAMDirectory.close() do it's best to release the resources it was
holding on to.

: This patch, while not doing much harm, is not really "best practice" in
: Java and the existing RAMDirectory close() was correct.

Why is this not a best practice?  (or do you mean that in a "not
particularly good" sense instead of "bad" sense?)



-Hoss


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