You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucenenet.apache.org by sy...@apache.org on 2012/06/14 12:19:14 UTC
svn commit: r1350178 -
/incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
Author: synhershko
Date: Thu Jun 14 10:19:13 2012
New Revision: 1350178
URL: http://svn.apache.org/viewvc?rev=1350178&view=rev
Log:
Allow for opening FSDirectory with a string
A proposed change - I'm tired of creating a DirInfo object all the time, doesn't really add any value
Modified:
incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
Modified: incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
URL: http://svn.apache.org/viewvc/incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs?rev=1350178&r1=1350177&r2=1350178&view=diff
==============================================================================
--- incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs (original)
+++ incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs Thu Jun 14 10:19:13 2012
@@ -210,6 +210,31 @@ namespace Lucene.Net.Store
}
}
}
+
+ /// <summary>Creates an FSDirectory instance, trying to pick the
+ /// best implementation given the current environment.
+ /// The directory returned uses the <see cref="NativeFSLockFactory" />.
+ ///
+ /// <p/>Currently this returns <see cref="SimpleFSDirectory" /> as
+ /// NIOFSDirectory is currently not supported.
+ ///
+ /// <p/><b>NOTE</b>: this method may suddenly change which
+ /// implementation is returned from release to release, in
+ /// the event that higher performance defaults become
+ /// possible; if the precise implementation is important to
+ /// your application, please instantiate it directly,
+ /// instead. On 64 bit systems, it may also good to
+ /// return <see cref="MMapDirectory" />, but this is disabled
+ /// because of officially missing unmap support in Java.
+ /// For optimal performance you should consider using
+ /// this implementation on 64 bit JVMs.
+ ///
+ /// <p/>See <a href="#subclasses">above</a>
+ /// </summary>
+ public static FSDirectory Open(string path)
+ {
+ return Open(new DirectoryInfo(path), null);
+ }
/// <summary>Creates an FSDirectory instance, trying to pick the
/// best implementation given the current environment.
Fwd: svn commit: r1350178 - /incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
Posted by Itamar Syn-Hershko <it...@code972.com>.
This is a proposed change
I got really tired of writing unnecessary code all the time
when opening FSDirectory
---------- Forwarded message ----------
From: <sy...@apache.org>
Date: Thu, Jun 14, 2012 at 1:19 PM
Subject: svn commit: r1350178 - /incubator/
lucene.net/trunk/src/core/Store/FSDirectory.cs
To: lucene-net-commits@lucene.apache.org
Author: synhershko
Date: Thu Jun 14 10:19:13 2012
New Revision: 1350178
URL: http://svn.apache.org/viewvc?rev=1350178&view=rev
Log:
Allow for opening FSDirectory with a string
A proposed change - I'm tired of creating a DirInfo object all the time,
doesn't really add any value
Modified:
incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
Modified: incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
URL:
http://svn.apache.org/viewvc/incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs?rev=1350178&r1=1350177&r2=1350178&view=diff
==============================================================================
--- incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs (original)
+++ incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs Thu Jun 14
10:19:13 2012
@@ -210,6 +210,31 @@ namespace Lucene.Net.Store
}
}
}
+
+ /// <summary>Creates an FSDirectory instance, trying to
pick the
+ /// best implementation given the current environment.
+ /// The directory returned uses the <see
cref="NativeFSLockFactory" />.
+ ///
+ /// <p/>Currently this returns <see
cref="SimpleFSDirectory" /> as
+ /// NIOFSDirectory is currently not supported.
+ ///
+ /// <p/><b>NOTE</b>: this method may suddenly change which
+ /// implementation is returned from release to release, in
+ /// the event that higher performance defaults become
+ /// possible; if the precise implementation is important to
+ /// your application, please instantiate it directly,
+ /// instead. On 64 bit systems, it may also good to
+ /// return <see cref="MMapDirectory" />, but this is
disabled
+ /// because of officially missing unmap support in Java.
+ /// For optimal performance you should consider using
+ /// this implementation on 64 bit JVMs.
+ ///
+ /// <p/>See <a href="#subclasses">above</a>
+ /// </summary>
+ public static FSDirectory Open(string path)
+ {
+ return Open(new DirectoryInfo(path), null);
+ }
/// <summary>Creates an FSDirectory instance, trying to pick
the
/// best implementation given the current environment.
Re: svn commit: r1350178 - /incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
Posted by Troy Howard <th...@gmail.com>.
Well, yeah, that's a lot more than I realized.. My assumption was that
there wouldn't be more FileInfo objects created than there were files
created, which is not that many. Whatever is doing that should be
re-written to pass the obj vs recreating it.
-T
On Thu, Jun 14, 2012 at 3:57 PM, Christopher Currens
<cu...@gmail.com> wrote:
> They're used more than it looks like they are. One of the largest ways
> they're used in the Store namespace is *to create FileStream
> objects*...which is such a waste of an allocation. A small test program I
> wrote created ~1000 FileInfo objects every minute, most of that thanks to
> merging. Considering the other object allocations in the library, though,
> it's unlikely removing them *alone* will do anything measurable to
> performance. But I disagree that it shouldn't be a concern. That kind of
> attitude will kill our performance if applied to other areas of the code.
>
>
> Thanks,
> Christopher
>
> On Thu, Jun 14, 2012 at 2:23 PM, Troy Howard <th...@gmail.com> wrote:
>
>> +1 on the string vs DirectoryInfo overload, Iatmar
>>
>> Re: Comments and JVM, I'd suggest editing the .NET comments to remove
>> Java specific recommendations/concerns. We're not running on the JVM,
>> so consumes of our code don't need that info.
>>
>> Re: GC pressure on the File/DirectoryInfo objects.. There's so few of
>> them it really doesn't make a difference. That aspect of their usage
>> should not be a concern. A greater concern is that they don't really
>> support the full range of Win32API file access (eg long paths, etc).
>>
>> Thanks,
>> Troy
>>
>>
>> On Thu, Jun 14, 2012 at 9:39 AM, Christopher Currens
>> <cu...@gmail.com> wrote:
>> > That comment is correct. We don't have support for NIOFSDirectory in
>> .NET
>> > (and the JVM support for it in windows has a major bug). We also don't
>> > support MMapDirectory, because we haven't bothered to support it yet. I
>> > think digy ported it once, but it didn't add any speed benefits, and was
>> > actually fairly slower than the FSDirectory classes.
>> >
>> > It wasn't that long ago that we had the string constructors for Directory
>> > classes. I think they were in the java code and then replace with File
>> > (DirectoryInfo for .NET). I've always hated passing in DirectoryInfo,
>> and
>> > I don't really understand why it's in the code. It doesn't seem to give
>> > much added benefit, if any. You can pass a string that is a path to an
>> > existing file and it will still create the DirectoryInfo object. I would
>> > think (but don't know) it would be better for performance to *not* use
>> the
>> > File and Directory classes (I'm actually not sure how deep the usages of
>> > these classes go, so I'm not sure what difference it would make), since
>> it
>> > results in added pressure on the GC with the extra object creations.
>> >
>> > +1 to this.
>> >
>> > On Thu, Jun 14, 2012 at 5:25 AM, Itamar Syn-Hershko <itamar@code972.com
>> >wrote:
>> >
>> >> I'd assume so, at least partially
>> >>
>> >> I just copy-pasted from one method below
>> >>
>> >> On Thu, Jun 14, 2012 at 2:52 PM, Stefan Bodewig <bo...@apache.org>
>> >> wrote:
>> >>
>> >> > On 2012-06-14, <sy...@apache.org> wrote:
>> >> >
>> >> > >> /// <p/>Currently this returns <see
>> >> > cref="SimpleFSDirectory" /> as
>> >> > >> /// NIOFSDirectory is currently not supported.
>> >> > >> ///
>> >> > >> /// <p/><b>NOTE</b>: this method may suddenly change
>> >> which
>> >> > >> /// implementation is returned from release to
>> release,
>> >> in
>> >> > >> /// the event that higher performance defaults become
>> >> > >> /// possible; if the precise implementation is
>> important
>> >> to
>> >> > >> /// your application, please instantiate it directly,
>> >> > >> /// instead. On 64 bit systems, it may also good to
>> >> > >> /// return <see cref="MMapDirectory" />, but this is
>> >> > disabled
>> >> > >> /// because of officially missing unmap support in
>> Java.
>> >> > >> /// For optimal performance you should consider using
>> >> > >> /// this implementation on 64 bit JVMs.
>> >> >
>> >> > Does this apply to the .NET code?
>> >> >
>> >> > Stefan
>> >> >
>> >> >
>> >>
>>
Re: svn commit: r1350178 - /incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
Posted by Christopher Currens <cu...@gmail.com>.
They're used more than it looks like they are. One of the largest ways
they're used in the Store namespace is *to create FileStream
objects*...which is such a waste of an allocation. A small test program I
wrote created ~1000 FileInfo objects every minute, most of that thanks to
merging. Considering the other object allocations in the library, though,
it's unlikely removing them *alone* will do anything measurable to
performance. But I disagree that it shouldn't be a concern. That kind of
attitude will kill our performance if applied to other areas of the code.
Thanks,
Christopher
On Thu, Jun 14, 2012 at 2:23 PM, Troy Howard <th...@gmail.com> wrote:
> +1 on the string vs DirectoryInfo overload, Iatmar
>
> Re: Comments and JVM, I'd suggest editing the .NET comments to remove
> Java specific recommendations/concerns. We're not running on the JVM,
> so consumes of our code don't need that info.
>
> Re: GC pressure on the File/DirectoryInfo objects.. There's so few of
> them it really doesn't make a difference. That aspect of their usage
> should not be a concern. A greater concern is that they don't really
> support the full range of Win32API file access (eg long paths, etc).
>
> Thanks,
> Troy
>
>
> On Thu, Jun 14, 2012 at 9:39 AM, Christopher Currens
> <cu...@gmail.com> wrote:
> > That comment is correct. We don't have support for NIOFSDirectory in
> .NET
> > (and the JVM support for it in windows has a major bug). We also don't
> > support MMapDirectory, because we haven't bothered to support it yet. I
> > think digy ported it once, but it didn't add any speed benefits, and was
> > actually fairly slower than the FSDirectory classes.
> >
> > It wasn't that long ago that we had the string constructors for Directory
> > classes. I think they were in the java code and then replace with File
> > (DirectoryInfo for .NET). I've always hated passing in DirectoryInfo,
> and
> > I don't really understand why it's in the code. It doesn't seem to give
> > much added benefit, if any. You can pass a string that is a path to an
> > existing file and it will still create the DirectoryInfo object. I would
> > think (but don't know) it would be better for performance to *not* use
> the
> > File and Directory classes (I'm actually not sure how deep the usages of
> > these classes go, so I'm not sure what difference it would make), since
> it
> > results in added pressure on the GC with the extra object creations.
> >
> > +1 to this.
> >
> > On Thu, Jun 14, 2012 at 5:25 AM, Itamar Syn-Hershko <itamar@code972.com
> >wrote:
> >
> >> I'd assume so, at least partially
> >>
> >> I just copy-pasted from one method below
> >>
> >> On Thu, Jun 14, 2012 at 2:52 PM, Stefan Bodewig <bo...@apache.org>
> >> wrote:
> >>
> >> > On 2012-06-14, <sy...@apache.org> wrote:
> >> >
> >> > >> /// <p/>Currently this returns <see
> >> > cref="SimpleFSDirectory" /> as
> >> > >> /// NIOFSDirectory is currently not supported.
> >> > >> ///
> >> > >> /// <p/><b>NOTE</b>: this method may suddenly change
> >> which
> >> > >> /// implementation is returned from release to
> release,
> >> in
> >> > >> /// the event that higher performance defaults become
> >> > >> /// possible; if the precise implementation is
> important
> >> to
> >> > >> /// your application, please instantiate it directly,
> >> > >> /// instead. On 64 bit systems, it may also good to
> >> > >> /// return <see cref="MMapDirectory" />, but this is
> >> > disabled
> >> > >> /// because of officially missing unmap support in
> Java.
> >> > >> /// For optimal performance you should consider using
> >> > >> /// this implementation on 64 bit JVMs.
> >> >
> >> > Does this apply to the .NET code?
> >> >
> >> > Stefan
> >> >
> >> >
> >>
>
Re: svn commit: r1350178 - /incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
Posted by Troy Howard <th...@gmail.com>.
+1 on the string vs DirectoryInfo overload, Iatmar
Re: Comments and JVM, I'd suggest editing the .NET comments to remove
Java specific recommendations/concerns. We're not running on the JVM,
so consumes of our code don't need that info.
Re: GC pressure on the File/DirectoryInfo objects.. There's so few of
them it really doesn't make a difference. That aspect of their usage
should not be a concern. A greater concern is that they don't really
support the full range of Win32API file access (eg long paths, etc).
Thanks,
Troy
On Thu, Jun 14, 2012 at 9:39 AM, Christopher Currens
<cu...@gmail.com> wrote:
> That comment is correct. We don't have support for NIOFSDirectory in .NET
> (and the JVM support for it in windows has a major bug). We also don't
> support MMapDirectory, because we haven't bothered to support it yet. I
> think digy ported it once, but it didn't add any speed benefits, and was
> actually fairly slower than the FSDirectory classes.
>
> It wasn't that long ago that we had the string constructors for Directory
> classes. I think they were in the java code and then replace with File
> (DirectoryInfo for .NET). I've always hated passing in DirectoryInfo, and
> I don't really understand why it's in the code. It doesn't seem to give
> much added benefit, if any. You can pass a string that is a path to an
> existing file and it will still create the DirectoryInfo object. I would
> think (but don't know) it would be better for performance to *not* use the
> File and Directory classes (I'm actually not sure how deep the usages of
> these classes go, so I'm not sure what difference it would make), since it
> results in added pressure on the GC with the extra object creations.
>
> +1 to this.
>
> On Thu, Jun 14, 2012 at 5:25 AM, Itamar Syn-Hershko <it...@code972.com>wrote:
>
>> I'd assume so, at least partially
>>
>> I just copy-pasted from one method below
>>
>> On Thu, Jun 14, 2012 at 2:52 PM, Stefan Bodewig <bo...@apache.org>
>> wrote:
>>
>> > On 2012-06-14, <sy...@apache.org> wrote:
>> >
>> > >> /// <p/>Currently this returns <see
>> > cref="SimpleFSDirectory" /> as
>> > >> /// NIOFSDirectory is currently not supported.
>> > >> ///
>> > >> /// <p/><b>NOTE</b>: this method may suddenly change
>> which
>> > >> /// implementation is returned from release to release,
>> in
>> > >> /// the event that higher performance defaults become
>> > >> /// possible; if the precise implementation is important
>> to
>> > >> /// your application, please instantiate it directly,
>> > >> /// instead. On 64 bit systems, it may also good to
>> > >> /// return <see cref="MMapDirectory" />, but this is
>> > disabled
>> > >> /// because of officially missing unmap support in Java.
>> > >> /// For optimal performance you should consider using
>> > >> /// this implementation on 64 bit JVMs.
>> >
>> > Does this apply to the .NET code?
>> >
>> > Stefan
>> >
>> >
>>
Re: svn commit: r1350178 - /incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
Posted by Christopher Currens <cu...@gmail.com>.
That comment is correct. We don't have support for NIOFSDirectory in .NET
(and the JVM support for it in windows has a major bug). We also don't
support MMapDirectory, because we haven't bothered to support it yet. I
think digy ported it once, but it didn't add any speed benefits, and was
actually fairly slower than the FSDirectory classes.
It wasn't that long ago that we had the string constructors for Directory
classes. I think they were in the java code and then replace with File
(DirectoryInfo for .NET). I've always hated passing in DirectoryInfo, and
I don't really understand why it's in the code. It doesn't seem to give
much added benefit, if any. You can pass a string that is a path to an
existing file and it will still create the DirectoryInfo object. I would
think (but don't know) it would be better for performance to *not* use the
File and Directory classes (I'm actually not sure how deep the usages of
these classes go, so I'm not sure what difference it would make), since it
results in added pressure on the GC with the extra object creations.
+1 to this.
On Thu, Jun 14, 2012 at 5:25 AM, Itamar Syn-Hershko <it...@code972.com>wrote:
> I'd assume so, at least partially
>
> I just copy-pasted from one method below
>
> On Thu, Jun 14, 2012 at 2:52 PM, Stefan Bodewig <bo...@apache.org>
> wrote:
>
> > On 2012-06-14, <sy...@apache.org> wrote:
> >
> > >> /// <p/>Currently this returns <see
> > cref="SimpleFSDirectory" /> as
> > >> /// NIOFSDirectory is currently not supported.
> > >> ///
> > >> /// <p/><b>NOTE</b>: this method may suddenly change
> which
> > >> /// implementation is returned from release to release,
> in
> > >> /// the event that higher performance defaults become
> > >> /// possible; if the precise implementation is important
> to
> > >> /// your application, please instantiate it directly,
> > >> /// instead. On 64 bit systems, it may also good to
> > >> /// return <see cref="MMapDirectory" />, but this is
> > disabled
> > >> /// because of officially missing unmap support in Java.
> > >> /// For optimal performance you should consider using
> > >> /// this implementation on 64 bit JVMs.
> >
> > Does this apply to the .NET code?
> >
> > Stefan
> >
> >
>
Re: svn commit: r1350178 - /incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
Posted by Itamar Syn-Hershko <it...@code972.com>.
I'd assume so, at least partially
I just copy-pasted from one method below
On Thu, Jun 14, 2012 at 2:52 PM, Stefan Bodewig <bo...@apache.org> wrote:
> On 2012-06-14, <sy...@apache.org> wrote:
>
> >> /// <p/>Currently this returns <see
> cref="SimpleFSDirectory" /> as
> >> /// NIOFSDirectory is currently not supported.
> >> ///
> >> /// <p/><b>NOTE</b>: this method may suddenly change which
> >> /// implementation is returned from release to release, in
> >> /// the event that higher performance defaults become
> >> /// possible; if the precise implementation is important to
> >> /// your application, please instantiate it directly,
> >> /// instead. On 64 bit systems, it may also good to
> >> /// return <see cref="MMapDirectory" />, but this is
> disabled
> >> /// because of officially missing unmap support in Java.
> >> /// For optimal performance you should consider using
> >> /// this implementation on 64 bit JVMs.
>
> Does this apply to the .NET code?
>
> Stefan
>
>
Re: svn commit: r1350178 - /incubator/lucene.net/trunk/src/core/Store/FSDirectory.cs
Posted by Stefan Bodewig <bo...@apache.org>.
On 2012-06-14, <sy...@apache.org> wrote:
>> /// <p/>Currently this returns <see cref="SimpleFSDirectory" /> as
>> /// NIOFSDirectory is currently not supported.
>> ///
>> /// <p/><b>NOTE</b>: this method may suddenly change which
>> /// implementation is returned from release to release, in
>> /// the event that higher performance defaults become
>> /// possible; if the precise implementation is important to
>> /// your application, please instantiate it directly,
>> /// instead. On 64 bit systems, it may also good to
>> /// return <see cref="MMapDirectory" />, but this is disabled
>> /// because of officially missing unmap support in Java.
>> /// For optimal performance you should consider using
>> /// this implementation on 64 bit JVMs.
Does this apply to the .NET code?
Stefan