Wlad,
Comments below.
vvaintroub@stripped wrote:
[...]
> ChangeSet@stripped, 2008-04-07 15:49:58+02:00, vvaintroub@wva. +2 -0
> WL#3998 - Falcon positioned IO.
> Use the native windows functionality to do positioned IO
> (pass the offset to ReadFile/WriteFile in the OVERLAPPED parameter)
The file is not opened with FILE_FLAG_OVERLAPPED, we should therefore assume write
operations will completed upon return, i.e. a synchronous write, correct?
> This allows to eliminate SetFilePointerEx/SyncObject locks around IOs.
>
> storage/falcon/IO.cpp@stripped, 2008-04-07 15:49:54+02:00, vvaintroub@wva. +25 -0
> WL#3998 - Falcon positioned IO.
> Use the native windows functionality to do positioned IO
> (pass the offset to ReadFile/WriteFile in the OVERLAPPED parameter)
>
> This allows to eliminate SetFilePointerEx/SyncObject locks around IOs.
>
> storage/falcon/SerialLogFile.cpp@stripped, 2008-04-07 15:49:55+02:00, vvaintroub@wva.
> +11 -19
> WL#3998 - Falcon positioned IO.
> Use the native windows functionality to do positioned IO
> (pass the offset to ReadFile/WriteFile in the OVERLAPPED parameter)
>
> This allows to eliminate SetFilePointerEx/SyncObject locks around IOs.
>
> diff -Nrup a/storage/falcon/IO.cpp b/storage/falcon/IO.cpp
> --- a/storage/falcon/IO.cpp 2008-03-27 07:09:04 +01:00
> +++ b/storage/falcon/IO.cpp 2008-04-07 15:49:54 +02:00
> @@ -500,6 +500,17 @@ int IO::pread(int64 offset, int length,
>
> #if defined(HAVE_PREAD) && !defined(HAVE_BROKEN_PREAD)
> ret = ::pread (fileId, buffer, length, offset);
> +#elif defined(_WIN32)
> + HANDLE hFile = (HANDLE)_get_osfhandle(fileId);
> + OVERLAPPED overlapped = {0};
> + LARGE_INTEGER pos;
> +
> + pos.QuadPart = offset;
> + overlapped.Offset = pos.LowPart;
> + overlapped.OffsetHigh = pos.HighPart;
> +
> + if (!ReadFile(hFile, buffer, length, (DWORD *) &ret, &overlapped))
> + ret = -1;
> #else
> Sync sync (&syncObject, "IO::pread");
> sync.lock (Exclusive);
> @@ -532,6 +543,20 @@ int IO::pwrite(int64 offset, int length,
>
> #if defined(HAVE_PREAD) && !defined(HAVE_BROKEN_PREAD)
> ret = ::pwrite (fileId, buffer, length, offset);
> +#elif defined(_WIN32)
> +
> + ASSERT(length > 0);
> +
> + HANDLE hFile = (HANDLE)_get_osfhandle(fileId);
> + OVERLAPPED overlapped = {0};
> + LARGE_INTEGER pos;
> +
> + pos.QuadPart = offset;
> + overlapped.Offset = pos.LowPart;
> + overlapped.OffsetHigh = pos.HighPart;
> +
> + if (!WriteFile(hFile, buffer, length, (DWORD *)&ret, &overlapped))
> + ret = -1;
> #else
> Sync sync (&syncObject, "IO::pwrite");
> sync.lock (Exclusive);
> diff -Nrup a/storage/falcon/SerialLogFile.cpp b/storage/falcon/SerialLogFile.cpp
> --- a/storage/falcon/SerialLogFile.cpp 2008-03-11 16:16:31 +01:00
> +++ b/storage/falcon/SerialLogFile.cpp 2008-04-07 15:49:55 +02:00
> @@ -179,22 +179,15 @@ void SerialLogFile::write(int64 position
> priority.schedule(PRIORITY_HIGH);
>
> #ifdef _WIN32
> -
> - Sync sync(&syncObject, "SerialLogFile::write");
> - sync.lock(Exclusive);
> -
Before this change, SerialLogFile::write() was serialized by an exclusive lock on
SerialLogFile::syncObject. Now that this lock is removed, are we assured that
SerialLogFile::write() is still protected?
I see that access to the serial log block being written is controlled by the mutex lock in
SerialLog::flush() and overflowFlush(), so we won't have multiple instances of
SerialLogFile::write() called on top of each other.
However, SeriaLog::copyClone() and SerialLogFile::zap() also call ::write(). Is this safe?
> - if (position != offset)
> - {
> - LARGE_INTEGER pos;
> - pos.QuadPart = position;
> -
> - if (!SetFilePointerEx(handle, pos, NULL, FILE_BEGIN))
> - throw SQLError(IO_ERROR, "serial log SetFilePointerEx failed with %d",
> GetLastError());
> - }
> + LARGE_INTEGER pos;
> + pos.QuadPart = position;
> + OVERLAPPED overlapped = {0};
> + overlapped.Offset = pos.LowPart;
> + overlapped.OffsetHigh = pos.HighPart;
>
> DWORD ret;
>
> - if (!WriteFile(handle, data, effectiveLength, &ret, NULL))
> + if (!WriteFile(handle, data, effectiveLength, &ret, &overlapped))
> {
> int lastError = GetLastError();
>
> @@ -259,18 +252,17 @@ uint32 SerialLogFile::read(int64 positio
> priority.schedule(PRIORITY_HIGH);
>
> #ifdef _WIN32
> - Sync sync(&syncObject, "SerialLogFile::read");
> - sync.lock(Exclusive);
> +
Similarly, SerialLogFile::read() was serialized by an exclusive lock on
SerialLogFile::syncObject. Now that this lock is removed, are we assured there won't be
concurrent calls to SerialLogFile::read()?
> ASSERT(position < writePoint || writePoint == 0);
> LARGE_INTEGER pos;
> pos.QuadPart = position;
> -
> - if (!SetFilePointerEx(handle, pos, NULL, FILE_BEGIN))
> - throw SQLError(IO_ERROR, "serial log SetFilePointer failed with %d",
> GetLastError());
> + OVERLAPPED overlapped = {0};
> + overlapped.Offset = pos.LowPart;
> + overlapped.OffsetHigh = pos.HighPart;
>
> DWORD ret;
>
> - if (!ReadFile(handle, data, effectiveLength, &ret, NULL))
> + if (!ReadFile(handle, data, effectiveLength, &ret, &overlapped))
> throw SQLError(IO_ERROR, "serial log ReadFile failed with %d", GetLastError());
>
> offset = position + effectiveLength;
>
