Skip to content

Commit

Permalink
get rid of incorrect information about lock-free implementation; It w…
Browse files Browse the repository at this point in the history
…as based on spinlocks and it is definitely has nothing common with lock-free
  • Loading branch information
kolomenkin committed May 23, 2022
1 parent 73e9ac5 commit d7b44c4
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 36 deletions.
22 changes: 11 additions & 11 deletions LineReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ std::optional<std::string_view> CMappingLineReader::GetNextLine()
//////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////

CLockFreeLineReader::CLockFreeLineReader()
CSpinlockLineReader::CSpinlockLineReader()
{
this->_buffer1.Allocate(ReadBufferSize);
if (this->_buffer1.ptr != nullptr)
Expand All @@ -370,7 +370,7 @@ CLockFreeLineReader::CLockFreeLineReader()
}
}

bool CLockFreeLineReader::Open(const wchar_t* const filename)
bool CSpinlockLineReader::Open(const wchar_t* const filename)
{
if (this->_buffer1.ptr == nullptr || filename == nullptr)
{
Expand All @@ -389,32 +389,32 @@ bool CLockFreeLineReader::Open(const wchar_t* const filename)
this->_bufferData = std::string_view(this->_buffer1.ptr, 0);
this->_firstBufferIsActive = true;

const bool initLockFreeOk = this->_file.LockFreecInit();
if (!initLockFreeOk)
const bool initSpinlockOk = this->_file.SpinlockInit();
if (!initSpinlockOk)
{
this->_file.Close();
return false;
}

const bool readStartOk = this->_file.LockFreeReadStart(this->_buffer2.ptr + ReadBufferOffset, ReadChunkSize);
const bool readStartOk = this->_file.SpinlockReadStart(this->_buffer2.ptr + ReadBufferOffset, ReadChunkSize);
if (!readStartOk)
{
this->_file.LockFreecClean();
this->_file.SpinlockClean();
this->_file.Close();
return false;
}

return true;
}

void CLockFreeLineReader::Close()
void CSpinlockLineReader::Close()
{
this->_file.LockFreecClean();
this->_file.SpinlockClean();
this->_file.Close();
}

__declspec(noinline) // noinline is added to help CPU profiling in release version
std::optional<std::string_view> CLockFreeLineReader::GetNextLine()
std::optional<std::string_view> CSpinlockLineReader::GetNextLine()
{
if (this->_buffer1.ptr == nullptr)
{
Expand Down Expand Up @@ -446,15 +446,15 @@ std::optional<std::string_view> CLockFreeLineReader::GetNextLine()
memcpy(newDataBufferPtr, this->_bufferData.data(), prefixLength);

size_t readBytes = 0;
const bool readCompleteOk = this->_file.LockFreeReadWait(readBytes);
const bool readCompleteOk = this->_file.SpinlockReadWait(readBytes);
if (!readCompleteOk)
{
// Previous reading failed
return {};
}

// Read missing data:
const bool readOk = this->_file.LockFreeReadStart(currentBuffer.ptr + ReadBufferOffset, ReadChunkSize);
const bool readOk = this->_file.SpinlockReadStart(currentBuffer.ptr + ReadBufferOffset, ReadChunkSize);
if (!readOk)
{
// New reading failed
Expand Down
6 changes: 3 additions & 3 deletions LineReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ class CMappingLineReader

//////////////////////////////////////////////////////////////////////////

// Implementation with 2 buffers and separate thread for sync calls to ReadFile(); synchronization is done without Windows EVENTs (lock free)
class CLockFreeLineReader
// Implementation with 2 buffers and separate thread for sync calls to ReadFile(); synchronization is done without Windows EVENTs (manual spin locks)
class CSpinlockLineReader
{
public:
CLockFreeLineReader();
CSpinlockLineReader();

bool Open(const wchar_t* const filename);
void Close();
Expand Down
2 changes: 1 addition & 1 deletion LogReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class CLogReader final
#if 0
CAsyncLineReader _lineReader;
#else
CLockFreeLineReader _lineReader;
CSpinlockLineReader _lineReader;
#endif
#endif
CCharBuffer _pattern;
Expand Down
14 changes: 7 additions & 7 deletions ScanFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ bool CScanFile::Open(const wchar_t* const filename, const bool asyncMode)

void CScanFile::Close()
{
this->LockFreecClean();
this->SpinlockClean();

if (this->_pViewOfFile != nullptr)
{
Expand Down Expand Up @@ -242,7 +242,7 @@ bool CScanFile::AsyncReadWait(size_t& readBytes)
/// Implementation of file API executed in a separate thread with the help of spinlocks
//////////////////////////////////////////////////////////////////////////

bool CScanFile::LockFreecInit()
bool CScanFile::SpinlockInit()
{
if (this->_hThread != nullptr)
{
Expand Down Expand Up @@ -284,7 +284,7 @@ bool CScanFile::LockFreecInit()
#endif

CScanFile* const that = static_cast<CScanFile*>(p);
that->LockFreeThreadProc();
that->SpinlockThreadProc();
_endthreadex(0);
return 0;
};
Expand All @@ -299,7 +299,7 @@ bool CScanFile::LockFreecInit()
return true;
}

void CScanFile::LockFreecClean()
void CScanFile::SpinlockClean()
{
if (this->_hThread != nullptr)
{
Expand All @@ -310,7 +310,7 @@ void CScanFile::LockFreecClean()
}

__declspec(noinline) // noinline is added to help CPU profiling in release version
bool CScanFile::LockFreeReadStart(char* const buffer, const size_t bufferLength)
bool CScanFile::SpinlockReadStart(char* const buffer, const size_t bufferLength)
{
if (this->_hThread == nullptr || this->_threadOperationInProgress)
{
Expand All @@ -332,7 +332,7 @@ bool CScanFile::LockFreeReadStart(char* const buffer, const size_t bufferLength)
}

__declspec(noinline) // noinline is added to help CPU profiling in release version
bool CScanFile::LockFreeReadWait(size_t& readBytes)
bool CScanFile::SpinlockReadWait(size_t& readBytes)
{
if (this->_hThread == nullptr || !this->_threadOperationInProgress)
{
Expand All @@ -359,7 +359,7 @@ bool CScanFile::LockFreeReadWait(size_t& readBytes)
return true;
}

void CScanFile::LockFreeThreadProc()
void CScanFile::SpinlockThreadProc()
{
while (true)
{
Expand Down
12 changes: 6 additions & 6 deletions ScanFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ class CScanFile
bool AsyncReadWait(size_t& readBytes);

// Current limitation: only one async operation can be in progress.
bool LockFreecInit();
void LockFreecClean();
bool LockFreeReadStart(char* const buffer, const size_t bufferLength);
bool LockFreeReadWait(size_t& readBytes);
void LockFreeThreadProc();
bool SpinlockInit();
void SpinlockClean();
bool SpinlockReadStart(char* const buffer, const size_t bufferLength);
bool SpinlockReadWait(size_t& readBytes);
void SpinlockThreadProc();

protected:
alignas(std::hardware_constructive_interference_size) // small speedup to get a bunch of variables into single cache line
Expand All @@ -49,7 +49,7 @@ class CScanFile
OVERLAPPED _asyncOverlapped = {};
bool _asyncOperationInProgress = false;

// Separate thread + Lock free:
// Separate thread + spin locks:
HANDLE _hThread = nullptr;

// for protection against wrong API usage, not for use in a worker thread, no memory protection:
Expand Down
2 changes: 1 addition & 1 deletion TestLineReaderLockFree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

namespace
{
# define CLineReader CLockFreeLineReader
# define CLineReader CSpinlockLineReader

const size_t MaxLogLineLength = 1024; // copy-pasted value from LineReader.cpp
}
Expand Down
6 changes: 3 additions & 3 deletions docs/implementation-notes-letter.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Test Task Implementation Notes
# Test Task Implementation Notes

## Decision progress

Expand Down Expand Up @@ -66,7 +66,7 @@ The latest application version takes 1.6 seconds to process test data while `gre

**ADDED:**
I also implemented reading the file in a separate thread. The file operation is synchronous,
synchronization between threads is done with the lock free loop (spinlock).
synchronization between threads is done using spinlocks.
It gave a total gain of 25% over the synchronous and asynchronous API solutions (total work time is 1.2 seconds).
This is 2 times faster than `grep`.

Expand All @@ -85,7 +85,7 @@ I kept all four implementations of reading files. You can switch them in code:
#if 0
CAsyncLineReader _lineReader;
#else
CLockFreeLineReader _lineReader;
CSpinlockLineReader _lineReader;
#endif
#endif
```
Expand Down
8 changes: 4 additions & 4 deletions docs/implementation-notes-letter.ru.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@

**ДОПОЛНЕНО:**
Реализовал также чтение файла в отдельном потоке. Файловая операция синхронная,
синхронизация с потоком lock free (spinlock).
синхронизация с рабочим потоком на спинлоках.
Получил общий выигрыш в 25% относительно решения на синхронном и асинхронном API (1.2 секунды).
Итого в 2 раза быстрее чем grep.

## Особенности реализации

В итоге у меня остались все три реализации чтения файлов.
Я оставил асинхронную версию как самую перспективную. Переключаются так:
В итоге у меня остались все четыре реализации чтения файлов.
Я оставил по умолчанию версию на спинлоках как самую перспективную. Переключаются так:

```cpp
#if 0
Expand All @@ -84,7 +84,7 @@
#if 0
CAsyncLineReader _lineReader;
#else
CLockFreeLineReader _lineReader;
CSpinlockLineReader _lineReader;
#endif
#endif
```
Expand Down

0 comments on commit d7b44c4

Please sign in to comment.