You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Christoph Goller <go...@detego-software.de> on 2003/11/19 18:30:46 UTC
Re: CompoundFileReader
Dmitry Serebrennikov schrieb:
> I put those in mostly to assure myself that I got things right. I think
> the key question is whether it possible to read part of another file. If
> not, I think that's fine. If yes, I think that's a problem.
>
> Dmitry.
>
Hi Dmitry,
I have been using my patch for a couple of weaks now and I think it is ok.
I kept the index bound checks, so that the original tests go through.
I would like to commit these changes. Did I answer all your questions
in my last email? Do you have any objections against commiting the patch?
Christoph
PS: patch is attached again.
Re: CompoundFileReader
Posted by Dmitry Serebrennikov <dm...@earthlink.net>.
+1
Looks like a great optimization.
Dmitry.
Christoph Goller wrote:
> Dmitry Serebrennikov schrieb:
>
>> I put those in mostly to assure myself that I got things right. I
>> think the key question is whether it possible to read part of another
>> file. If not, I think that's fine. If yes, I think that's a problem.
>>
>> Dmitry.
>>
>
> Hi Dmitry,
>
> I have been using my patch for a couple of weaks now and I think it is
> ok.
> I kept the index bound checks, so that the original tests go through.
> I would like to commit these changes. Did I answer all your questions
> in my last email? Do you have any objections against commiting the patch?
>
> Christoph
>
> PS: patch is attached again.
>
>------------------------------------------------------------------------
>
>Index: CompoundFileReader.java
>===================================================================
>RCS file: /home/cvs/jakarta-lucene/src/java/org/apache/lucene/index/CompoundFileReader.java,v
>retrieving revision 1.2
>diff -u -r1.2 CompoundFileReader.java
>--- CompoundFileReader.java 13 Oct 2003 14:18:04 -0000 1.2
>+++ CompoundFileReader.java 19 Nov 2003 13:06:40 -0000
>@@ -240,10 +240,9 @@
> final long length)
> throws IOException
> {
>- this.base = (InputStream) base.clone();
>+ this.base = base;
> this.fileOffset = fileOffset;
> this.length = length; // variable in the superclass
>- seekInternal(0); // position to the adjusted 0th byte
> }
>
> /** Expert: implements buffer refill. Reads bytes from the current
>@@ -255,7 +254,10 @@
> protected void readInternal(byte[] b, int offset, int len)
> throws IOException
> {
>- base.readBytes(b, offset, len);
>+ synchronized (base) {
>+ base.seek(fileOffset + getFilePointer());
>+ base.readBytes(b, offset, len);
>+ }
> }
>
> /** Expert: implements seek. Sets current position in this file, where
>@@ -269,29 +271,11 @@
>
> if (pos < 0)
> throw new IOException("Seek to a negative offset");
>-
>- base.seek(fileOffset + pos);
> }
>
> /** Closes the stream to futher operations. */
> public void close() throws IOException
>- {
>- base.close();
>- }
>+ {}
>
>- /** Returns a clone of this stream.
>- *
>- * <p>Clones of a stream access the same data, and are positioned at the same
>- * point as the stream they were cloned from.
>- *
>- * <p>Expert: Subclasses must ensure that clones may be positioned at
>- * different points in the input from each other and from the stream they
>- * were cloned from.
>- */
>- public Object clone() {
>- CSInputStream other = (CSInputStream) super.clone();
>- other.base = (InputStream) base.clone();
>- return other;
>- }
> }
> }
>
>
>
>------------------------------------------------------------------------
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
Re: CompoundFileReader
Posted by Doug Cutting <cu...@lucene.com>.
Christoph Goller wrote:
> I would like to commit these changes.
+1
Thanks,
Doug
---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
Re: CompoundFileReader
Posted by Christoph Goller <go...@detego-software.de>.
Otis Gospodnetic schrieb:
> Hello Christoph,
>
> I don't know enough to judge all changes you made with confidence, and
> don't have the code on this machine to apply the patch and look at it
> properly. However, it looks like seekInternal(long) should now be left
> with only the conditionals that can cause IOException, and nothing
> else. Why do we need to keep the body of the method then?
I discussed that already with Dmitry and he was not very passionate
regarding these conditionals. Finally I kept them since they were required
by Dmitry's test. But throwing an exception on seek is
not a behavior implemented by the other subclasses of InputStream.
I decided to remove the conditionals and slightly changed the unit test.
If more index checks are desirable, I would put them into InputStream.
Despite that I added a "read past EOF" check to CSInputStream.readInternal
to guarantee the same behavior as FSInputStream.
>
> I also see getFilePointer() call, but can't tell where that method is.
> I'll assume it's from the parent InputStream.
yes.
---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
Re: CompoundFileReader
Posted by Otis Gospodnetic <ot...@yahoo.com>.
Hello Christoph,
I don't know enough to judge all changes you made with confidence, and
don't have the code on this machine to apply the patch and look at it
properly. However, it looks like seekInternal(long) should now be left
with only the conditionals that can cause IOException, and nothing
else. Why do we need to keep the body of the method then?
I also see getFilePointer() call, but can't tell where that method is.
I'll assume it's from the parent InputStream.
Otis
--- Christoph Goller <go...@detego-software.de> wrote:
> Dmitry Serebrennikov schrieb:
> > I put those in mostly to assure myself that I got things right. I
> think
> > the key question is whether it possible to read part of another
> file. If
> > not, I think that's fine. If yes, I think that's a problem.
> >
> > Dmitry.
> >
>
> Hi Dmitry,
>
> I have been using my patch for a couple of weaks now and I think it
> is ok.
> I kept the index bound checks, so that the original tests go through.
> I would like to commit these changes. Did I answer all your questions
> in my last email? Do you have any objections against commiting the
> patch?
>
> Christoph
>
> PS: patch is attached again.
> > Index: CompoundFileReader.java
> ===================================================================
> RCS file:
>
/home/cvs/jakarta-lucene/src/java/org/apache/lucene/index/CompoundFileReader.java,v
> retrieving revision 1.2
> diff -u -r1.2 CompoundFileReader.java
> --- CompoundFileReader.java 13 Oct 2003 14:18:04 -0000 1.2
> +++ CompoundFileReader.java 19 Nov 2003 13:06:40 -0000
> @@ -240,10 +240,9 @@
> final long length)
> throws IOException
> {
> - this.base = (InputStream) base.clone();
> + this.base = base;
> this.fileOffset = fileOffset;
> this.length = length; // variable in the superclass
> - seekInternal(0); // position to the adjusted 0th
> byte
> }
>
> /** Expert: implements buffer refill. Reads bytes from the
> current
> @@ -255,7 +254,10 @@
> protected void readInternal(byte[] b, int offset, int len)
> throws IOException
> {
> - base.readBytes(b, offset, len);
> + synchronized (base) {
> + base.seek(fileOffset + getFilePointer());
> + base.readBytes(b, offset, len);
> + }
> }
>
> /** Expert: implements seek. Sets current position in this
> file, where
> @@ -269,29 +271,11 @@
>
> if (pos < 0)
> throw new IOException("Seek to a negative offset");
> -
> - base.seek(fileOffset + pos);
> }
>
> /** Closes the stream to futher operations. */
> public void close() throws IOException
> - {
> - base.close();
> - }
> + {}
>
> - /** Returns a clone of this stream.
> - *
> - * <p>Clones of a stream access the same data, and are
> positioned at the same
> - * point as the stream they were cloned from.
> - *
> - * <p>Expert: Subclasses must ensure that clones may be
> positioned at
> - * different points in the input from each other and from
> the stream they
> - * were cloned from.
> - */
> - public Object clone() {
> - CSInputStream other = (CSInputStream) super.clone();
> - other.base = (InputStream) base.clone();
> - return other;
> - }
> }
> }
>
> >
---------------------------------------------------------------------
> To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: lucene-dev-help@jakarta.apache.org
__________________________________
Do you Yahoo!?
Protect your identity with Yahoo! Mail AddressGuard
http://antispam.yahoo.com/whatsnewfree
---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org