[
https://issues.apache.org/jira/browse/LUCENE-2791?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12966486#action_12966486
]
Native Policeman edited comment on LUCENE-2791 at 12/3/10 6:11 AM:
-------------------------------------------------------------------
There are some violations:
bq. We can use Windows' overlapped IO to do pread() and avoid the performance
problems of SimpleFS/NIOFSDir.
Ahm you dont really use overlapped/async IO, ReadFile in your code only returns
when read operation is finished, so not async. Because when you do you have to
pass a flag: FILE_FLAG_OVERLAPPED. But I don't think we should really make it
async for now, that is too much stuff as Lucene does not really support it at
the moment.
- Currently you don't really need the OVERLAPPED structure at all, just pass
NULL: [http://msdn.microsoft.com/en-us/library/aa365467(VS.85).aspx], or is
there a reason?
- Do we need FILE_SHARE_WRITE?
- You use env->GetStringUTFChars, which is not really a problem as index
filenames are always ASCII only. The default methods in the Win32 API use the
system default charset (ANSI) for the arguments to OpenFile, which links to
"OpenFileA" in kernel32.dll. I would suggest to use the real UTF16 chars
(env->GetStringChars) and use OpenFileW method, which takes wchar instead of
char. The same applies to Linux Dir, but there I am not sure how encodings are
handled.
- Also the Format functions for GetLastError() results use ANSI per default. We
should use the *W method, too, and pass it to java as wchar.
- Exception handling: in native read, you call throwIOException, when
ReadFile() returns false. But 2 lines later you still call the method to copy
the local buffer to the java byte array. JNI docs say, that after an Exception
is in the thread's local exception, you should not call any JNI methods anymore
(expect some allowed ones). Also the copy operation is not really needed.
- As in contract to the native Linux directory we are using our own Handles and
not Java's FileDescriptor class to store the handle. If something goes wrong,
GC can never clean up the file and it stays open. So you should add a
finalizer, Java's internal File methods do that also (RandomAccessFile has a
finalizer that calls close on operating system file).
was (Author: nativepol):
There are some violations:
bq. We can use Windows' overlapped IO to do pread() and avoid the performance
problems of SimpleFS/NIOFSDir.
Ahm you dont really use overlapped/async IO, ReadFile in your code only returns
when read operation is finished, so not async. Because when you do you have to
pass a flag: FILE_FLAG_OVERLAPPED. But I don't think we should really make it
async for now, that is too much stuff as Lucene does not really support it at
the moment.
- Currently you don't really need the OVERLAPPED structure at all, just pass
NULL: [http://msdn.microsoft.com/en-us/library/aa365467(VS.85).aspx], or is
there a reason?
- Do we need FILE_SHARE_WRITE?
- You use env->GetStringUTFChars, which is not really a problem as index
filenames are always ASCII only. The default methods in the Win32 API use the
system default charset (ANSI) for the arguments to OpenFile, which liks to
"OpenFileA" in kernel32.dll. I would suggest to use the reaö UTF16 chars
(env->GetStringChars) and use OpenFileW method, which takes wchar instead of
char. The same applies to Linux Dir, but there I am not sure how encodings are
handled.
- Exception handling: in native read, you call throwIOException, when
ReadFile() returns false. But 2 lines later you still call the method to copy
the local buffer to the java byte array. JNI docs say, that after an Exception
is in the thread's local exception, you should not call any JNI methods anymore
(expect some allowed ones). Also the copy operation is not really needed.
- As in contract to the native Linux directory we are using our own Handles and
not Java's FileDescriptor class to store the handle. If something goes wrong,
GC can never clean up the file and it stays open. So you should add a
finalizer, Java's internal File methods do that also (RandomAccessFile has a
finalizer that calls close on operating system file).
> WindowsDirectory
> ----------------
>
> Key: LUCENE-2791
> URL: https://issues.apache.org/jira/browse/LUCENE-2791
> Project: Lucene - Java
> Issue Type: New Feature
> Components: Store
> Reporter: Robert Muir
> Attachments: LUCENE-2791.patch, LUCENE-2791.patch,
> WindowsDirectory.dll, WindowsDirectory_amd64.dll
>
>
> We can use Windows' overlapped IO to do pread() and avoid the performance
> problems of SimpleFS/NIOFSDir.
--
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: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
