Skip to content

PartFile: atomic-rename SavePartFile to cut per-tick disk I/O - #670

Merged
mrjimenez merged 1 commit into
amule-project:masterfrom
got3nks:fix/savepartfile-atomic-rename
May 22, 2026
Merged

PartFile: atomic-rename SavePartFile to cut per-tick disk I/O#670
mrjimenez merged 1 commit into
amule-project:masterfrom
got3nks:fix/savepartfile-atomic-rename

Conversation

@got3nks

@got3nks got3nks commented May 21, 2026

Copy link
Copy Markdown
Contributor

Fixes #669 (amuled wedge after ~12h on Stoatwblr's setup).

What

CPartFile::SavePartFile is called once per dirty partfile per save tick. The current implementation performs three full-size content copies of the .part.met file per call: pre-write BackupFile to .backup, delete the live .part.met, write a new one from scratch, delete .backup, then BackupFile the freshly-written .part.met to .part.met.bak as 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 in CPath::CloneFile -> wxCopyFile -> wxFile::Write -> __libc_write exactly 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:

  1. Write new content into .part.met.tmp.
  2. rename(.part.met, .part.met.bak) - metadata op, promotes the previous live file to long-term backup (overwriting any older .bak).
  3. 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.met is 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.bak on next start if the live .part.met goes missing.

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.
@got3nks
got3nks marked this pull request as ready for review May 21, 2026 22:28
@mrjimenez
mrjimenez merged commit 87f911c into amule-project:master May 22, 2026
12 checks passed
@got3nks
got3nks deleted the fix/savepartfile-atomic-rename branch May 22, 2026 13:43
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

new amuled wedge

2 participants