PartFile: atomic-rename SavePartFile to cut per-tick disk I/O - #670
Merged
mrjimenez merged 1 commit intoMay 22, 2026
Merged
Conversation
Old flow per dirty partfile, every save tick:
1. CPath::BackupFile(.met, ".backup") - full content copy
2. CPath::RemoveFile(.met)
3. Write new .met from scratch
4. CPath::RemoveFile(".backup") on success
5. CPath::BackupFile(.met, PARTMET_BAK_EXT) - second full content copy
Three full-size content copies per save, plus a window where the live
.part.met is absent on disk. On large sharers (hundreds of dirty
partfiles per timer tick) the aggregate keeps the main thread inside
boost::asio-blocking write() syscalls long enough for the EC dispatch
loop to starve - reported in issue amule-project#669 as amuled going catatonic
after ~12h of operation, with main-thread backtraces parked in
CPath::CloneFile -> wxCopyFile -> wxFile::Write.
New flow:
1. Write new content into .part.met.tmp
2. rename(.part.met, .part.met.bak) - metadata op, promotes
previous .met to long-term backup
3. rename(.part.met.tmp, .part.met) - atomic install
One content write plus two metadata renames per save. POSIX rename(2)
guarantees the target name is either fully old-content or fully
new-content at every observable moment, so crash safety is strictly
better than the old delete/create dance.
Catch blocks now clean up the tmp file before returning, and a sanity
check rejects a zero-length tmp (disk full / quota) without clobbering
the existing .part.met.
This was referenced May 21, 2026
Closed
got3nks
marked this pull request as ready for review
May 21, 2026 22:28
ngosang
pushed a commit
to ngosang/amule
that referenced
this pull request
Jul 28, 2026
…tching category (amule-project#670) Typing in the transfer-window text filter froze the GUI for seconds on a large queue -- ~10 s per keystroke with 10k downloads, with keystrokes stacking up (issue amule-project#669). SetFilterText() re-evaluated every model item and called ShowFile(file, visible) for each one. Every row that had to disappear went through RemoveItemData(), which does a full RebuildRowIndex() plus a RefreshFromRow() -- O(n) per removal, so O(n^2) for the pass. The shared-files filter is instant because it does the opposite: it clears the model and re-appends the passing items, finishing with a single FinishBulkLoad(). Give the download list the same shape. RebuildVisibleList() clears, appends every item passing the current category + text filter, and finishes with one FinishBulkLoad() (one SetItemCount, one index rebuild, one sort) -- O(n log n) and a single repaint. ShowFileList(), the filter and ChangeCategory() all route through it, so the identical O(n^2) in the category switch is fixed too and the three paths no longer duplicate the visibility loop. ShowFilesCount(diff) now delegates to SetFilesCount(count) so the rebuild can set the count absolutely without duplicating the label update. The two lists had also grown a filter each: identical m_filterText members, identical SetFilterText() bodies and byte-identical PassesTextFilter() implementations differing only in the parameter type. That now lives once in CMuleVirtualListCtrl -- m_filterText, SetFilterText() and a string-based MatchesFilter(), plus a RebuildFilteredView() hook each list overrides with its own rebuild. The base stays agnostic about what its rows represent. A clear-and-re-append rebuild renumbers every row, and the virtual control tracks selection and focus by row index, so both would be lost -- the incremental path kept them for free by leaving surviving rows untouched. CMuleVirtualListCtrl grows SaveSelection()/RestoreSelection() for this; SortList() already did the same save-and-reapply inline and now shares them, as does CSharedFilesCtrl::ShowFileList(), which fixes the pre-existing loss of selection when editing the shared-files filter.
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
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.
Fixes #669 (amuled wedge after ~12h on Stoatwblr's setup).
What
CPartFile::SavePartFileis called once per dirty partfile per save tick. The current implementation performs three full-size content copies of the.part.metfile per call: pre-writeBackupFileto.backup, delete the live.part.met, write a new one from scratch, delete.backup, thenBackupFilethe freshly-written.part.metto.part.met.bakas the long-term recovery file.On a sharer with hundreds of dirty partfiles per tick, the aggregate keeps amuled's main thread inside
write()syscalls long enough for the boost::asio EC dispatch loop to starve. Stoatwblr's gdb backtrace on #669 has the main thread parked inCPath::CloneFile -> wxCopyFile -> wxFile::Write -> __libc_writeexactly during this sequence. Individual writes complete in microseconds (per strace); the bottleneck is the loop length itself, which is why the wedge only manifests after the partfile count and dirty-tick frequency have grown enough hours into a session.How
Replace the delete/copy dance with an atomic-rename pattern:
.part.met.tmp.rename(.part.met, .part.met.bak)- metadata op, promotes the previous live file to long-term backup (overwriting any older.bak).rename(.part.met.tmp, .part.met)- atomic install of the new content.One content write plus two metadata renames per save. POSIX
rename(2)guarantees the target is either fully old-content or fully new-content at every observable moment, so crash safety is strictly better than the old delete/create window where.part.metis absent from disk.Catch blocks clean up the tmp file before returning false. A sanity check rejects a zero-length tmp (disk full / quota) without clobbering the existing
.part.met.Status
Draft - awaiting OP confirmation on the original report (#669) before promoting. No changes to load-path / recovery semantics;
LoadPartFile(from_backup=true)still picks up the.part.met.bakon next start if the live.part.metgoes missing.