lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shai Erera <ser...@gmail.com>
Subject Re: Turning IndexReader.isDeleted implementations to final
Date Sun, 28 Feb 2010 19:18:13 GMT
Reading around I think that Uwe is right. Adding final will only help
hotspot, but won't guarantee that not adding it will not result in inlining.
If we're sure that these classes really should be final, then let's do it.
Otherwise, we might as well not block ourselves, or someone else ...

Uwe, I didn't understand how Java 5 is related. Can you please educate me?
:)

Shai

On Sun, Feb 28, 2010 at 8:40 PM, Uwe Schindler <uwe@thetaphi.de> wrote:

> As far as I know, the HotSpot compiler does not really take care of final
> anymore. In the older java ages that was important, but even non-final
> methods are inlined by hotspot, if the compiler is sure that the class was
> not extended and so on. So making a method final just for inlining is no
> longer really needed. But with final you help hotspot more. Because of that,
> e.g. the collect methods in TFDC should be final and so on. But there is no
> requirement anymore. And Lucene 3.1 only runs with Java 5+, so who cares?
>
> -----
> Uwe Schindler
> H.-H.-Meier-Allee 63, D-28213 Bremen
> http://www.thetaphi.de
> eMail: uwe@thetaphi.de
>
> > -----Original Message-----
> > From: Michael McCandless [mailto:lucene@mikemccandless.com]
> > Sent: Sunday, February 28, 2010 7:30 PM
> > To: java-dev@lucene.apache.org
> > Subject: Re: Turning IndexReader.isDeleted implementations to final
> >
> > Sorry, I think the classes?  Not sure which others should be...
> >
> > Though it always spooks me out trying to decide if something should
> > really be final... these two are package private so in theory nobody
> > should be extending them, anyway, but if out there someone subclassed
> > them (mixing code into oal.index) to fix something and didn't post
> > back to us then we're slamming that door shut... all for likely tiny
> > (if at all) perf gains.  But then if we do make them final we would
> > then hear from any (if any) places that broke about what we should
> > fix/make extensible in our core impls..
> >
> > I think using final for classes like any of our analyzers makes alot
> > of sense.  There we explicitly want to state that you should go make
> > your own class if you need a custom analyzer.  Too many problems
> > surface if you subclass core analyzers.
> >
> > Also... making DirectoryReader final would prevent any sneaky
> > subclassing out there... which'd be good because I don't actually like
> > that DirectoryReader (and, MultiReader) even subclass IndexReader.  I
> > don't think they should, ie, I think it's bad that you can get
> > Multi*Enum, call isDeleted (doing a binary search each time).  You
> > should work per segment (use MultiSearcher) instead.  So tightening
> > these classes up for that possible future seems good?  Maybe we should
> > even deprecate them!
> >
> > So +1 to make these 2 final, and maybe also MultiReader, but not sure
> > which others...
> >
> > Mike
> >
> > On Sun, Feb 28, 2010 at 12:10 PM, Shai Erera <serera@gmail.com> wrote:
> > > What's ok? making the classes final or just the method declaration?
> > If
> > > classes, besides ReadOnlySegmentReader, which other impls do you
> > think can
> > > be made final (I'm not in front of the code)?
> > >
> > > On Sun, Feb 28, 2010 at 7:05 PM, Michael McCandless
> > > <lucene@mikemccandless.com> wrote:
> > >>
> > >> Seems OK I think?
> > >>
> > >> Mike
> > >>
> > >> On Sun, Feb 28, 2010 at 12:37 AM, Shai Erera <serera@gmail.com>
> > wrote:
> > >> > Hi
> > >> >
> > >> > Do you think it's worth to make some of the isDeleted method impls
> > >> > final,
> > >> > like in ReadOnlySegmentReader and (maybe) DirectoryReader? I'm
> > thinking
> > >> > the
> > >> > classes that are perceived as final could benefit from that, since
> > their
> > >> > impl could be inlined. Maybe just make these classes final
> > entirely?
> > >> >
> > >> > Shai
> > >> >
> > >>
> > >> --------------------------------------------------------------------
> > -
> > >> 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
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-dev-help@lucene.apache.org
>
>

Mime
View raw message