CFile: 64 KB userspace write buffer to coalesce metadata-save syscalls (#562) - #573
Merged
mrjimenez merged 1 commit intoMay 11, 2026
Merged
Conversation
amule-project#562) CFile::doWrite() was a thin wrapper over ::write(m_fd, ...), so every CFileDataIO::WriteTag / WriteString / Write call round-tripped through the kernel. For CKnownFileList::Save() with hundreds of thousands of files (each carrying ~10 sub-tags), this meant millions of small write() syscalls and a 9-minute shutdown freeze on Linux/HDD setups (slrslr's report on amule-project#562; gdb backtrace pinned the main thread inside __GI___libc_write → CFile::doWrite → CFileDataIO::WriteTag → CKnownFile::WriteToFile → CKnownFileList::Save; strace -c showed 100%% CPU time in write syscalls at ~26k/s). Add a 64 KB userspace write buffer to CFile. doWrite() accumulates into the buffer and flushes when full or when a single payload exceeds it. DrainWriteBuffer() is called from every path that needs the file's on-disk state to reflect preceding writes — Close, Flush, doSeek, doRead, GetPosition, GetLength, SetLength — so the buffer never holds bytes hostage past any observable boundary. ~CFile() calls Close(), so even an abandoned stack-allocated CFile drains. Read-only files bypass the buffer entirely (via m_canBuffer flag set in Open() based on OpenMode). This preserves the contract that writing to a read-opened file fails immediately at the call site — see FileDataIOTest's CFile.Constructor ASSERT_RAISES(..., file.WriteUInt8(0)). Multi-platform: identical code on Linux, macOS, Windows (mingw). The buffer is a fixed-size char[] member — 64 KB per open CFile, paid only when files are actually open. Typical aMule has a handful of open CFile objects at a time (.met files, log targets), so steady- state RSS impact is < 1 MB. Doesn't change wire semantics or file format. Any program reading a .met file produced by the patched daemon sees identical bytes — only the kernel-call pattern producing them changes. Existing unit tests pass on Mac (wxOSX) and Linux (wxGTK) — 9/9 ctest. FileDataIOTest covers the relevant Read / Write / Seek / SetLength / read-only / String / LargeFile paths across CFile and CMemFile.
|
I see at least no regression or obviously broken met files after 2 restarts with this PR ;-) Speed was fine for me even before. |
This was referenced May 11, 2026
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
Jul 24, 2026
…ect#573) * feat(amulegui): wire the Kad "More" search button over EC * chore(i18n): regenerate app catalogs for the moved Kad-search strings
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CFile::doWrite()was a thin wrapper over::write(m_fd, ...)— everyCFileDataIO::WriteTag/WriteString/Writecall round-tripped through the kernel. ForCKnownFileList::Save()with hundreds of thousands of files (each carrying ~10 sub-tags), this meant millions of smallwrite()syscalls and a 9-minute shutdown freeze on Linux/HDD setups (slrslr's report on #562, gdb backtrace pinned the main thread inside__GI___libc_write→CFile::doWrite→CFileDataIO::WriteTag→CKnownFile::WriteToFile→CKnownFileList::Save;strace -cshowed 100% CPU time inwritesyscalls at ~26k/s).Fix
Add a 64 KB userspace write buffer to
CFile.doWrite()accumulates into the buffer and flushes when full or when a single payload exceeds it.DrainWriteBuffer()is called from every path that needs the file's on-disk state to reflect preceding writes —Close,Flush,doSeek,doRead,GetPosition,GetLength,SetLength— so the buffer never holds bytes hostage past any observable boundary.Read-only files bypass the buffer entirely (via a
m_canBufferflag set inOpen()based onOpenMode). This preserves the contract that writing to a read-opened file fails immediately at the call site — seeFileDataIOTest'sCFile.ConstructorASSERT_RAISES(CIOFailureException, file.WriteUInt8(0)).Verification
FileDataIOTestcovers Read / Write / Seek / SetLength / Read-only / String / LargeFile acrossCFileandCMemFile). Both Mac (wxOSX) and Linux (wxGTK) full ctest passes — 9/9.CKnownFileList::Save()'s syscall count drops from O(millions) to O(thousands) (one syscall per filled 64 KB buffer instead of one per tag/string field).Caveats
Multi-platform: identical code path on Linux, macOS, Windows (mingw). The buffer is a fixed-size
char[]member — 64 KB per openCFileinstance, paid only when files are actually open. Typical aMule has a handful of openCFileobjects at a time (.metfiles, log targets), so the steady-state RSS impact is < 1 MB.The fix doesn't change wire semantics or file format. Any program reading a
.metfile produced by the patched daemon sees identical bytes — only the kernel-call pattern producing them changes.