Skip to content

SharedFilesCtrl: fix O(N²) GUI cascade on server disconnect (#302) - #561

Merged
mrjimenez merged 2 commits into
amule-project:masterfrom
got3nks:fix/sharedfilesctrl-update-cascade
May 11, 2026
Merged

SharedFilesCtrl: fix O(N²) GUI cascade on server disconnect (#302)#561
mrjimenez merged 2 commits into
amule-project:masterfrom
got3nks:fix/sharedfilesctrl-update-cascade

Conversation

@got3nks

@got3nks got3nks commented May 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #302. Mid-2026 retest by @mifritscher2 with a 10–100k-file shared library produced a gdb backtrace showing the actual root cause of the long-standing 100% CPU / GUI freeze on server disconnect (and on app exit, which calls Disconnect):

#0  wxListMainWindow::FindItem(long, unsigned long)
#1  CSharedFilesCtrl::UpdateItem(CKnownFile*)
#2  CKnownFile::SetPublishedED2K(bool)
#3  CSharedFileList::ClearED2KPublishInfo()
#4  CServerConnect::Disconnect()

ClearED2KPublishInfo() walks every shared file and calls SetPublishedED2K(false), which unconditionally fires Notify_SharedFilesUpdateItemCSharedFilesCtrl::UpdateItemFindItem(-1, ptr) (O(N) linear scan). N × O(N) ≈ 10¹⁰ on the main thread for a 100k-file library — a 10–15 min freeze, exactly matching the original report.

Fix

Two narrow, layered changes:

1. SetPublishedED2K early-return when value didn't change (src/KnownFile.cpp). On any server-disconnect, ~99.5% of shared files are already in the false state (server publish caps at 200 files per peer); they paid the full notify-into-GUI cost just to repaint identical content. Skipping the notify is a pure no-op visually since Notify_SharedFilesUpdateItem is a #ifndef AMULE_DAEMON GUI-repaint signal with no other subscribers.

2. Bulk-update API on CSharedFilesCtrl (src/SharedFilesCtrl.{h,cpp}, src/GuiEvents.{h,cpp}). For the genuinely-true→false transitions (≤200 per server) that still slip through Change 1, add BeginBulkUpdate() / EndBulkUpdate(): while set, UpdateItem() short-circuits; EndBulkUpdate() issues a single full Refresh(). Wired via the existing Notify_* dispatch (sync from main thread, FIFO-queued from workers), so CSharedFileList::ClearED2KPublishInfo (src/SharedFileList.cpp) can wrap its loop without daemon-side knowing about the GUI.

Together these turn ClearED2KPublishInfo from O(N²) into O(N) with a single trailing repaint — for 100k files, milliseconds instead of minutes.

Other Notify_SharedFilesUpdateItem callers were audited: the rest are single-file (rename, priority change, partfile pause/stop) or bounded-small (≤200 in OP_OFFERFILES); none enter the bulk-update window. The flag is only set during ClearED2KPublishInfo's loop, so the rest of the GUI behaves identically.

Test

Smoke-tested locally on macOS aMule.app:

  • Shared files list paints normally at startup.
  • Connect / disconnect / switch servers — no visible changes to the shared files view; nothing broken.
  • amuled builds clean (the daemon never enters the new code path; all bulk-update functions are #ifndef AMULE_DAEMON).

Reporter @mifritscher2 has the perfect natural reproducer (10–100k file library, can reliably trigger via server disconnect) and is positioned to validate the fix at scale.

got3nks added 2 commits May 9, 2026 23:16
CSharedFileList::ClearED2KPublishInfo() walks every shared file and
calls SetPublishedED2K(false) regardless of the current value. With
~99.5% of any user's shared files in the false state at any moment
(server publish caps at 200 files per peer), the false→false majority
was paying the full notify-into-GUI cost: dispatch, FindItem (linear
scan O(N) over the wxListCtrl) and a queued repaint, all to repaint
identical content.

For users with thousands of shared files in CSharedFilesCtrl this turns
ClearED2KPublishInfo from O(N²) into O(N+K) where K ≤ 200, dropping the
freeze on server disconnect / app exit from minutes to milliseconds.

The notify is purely a GUI repaint signal (#ifndef AMULE_DAEMON) with
no other subscribers, so skipping it when the boolean didn't change
is a pure no-op visually.

See amule-project#302 for the original 100% CPU report and the gdb backtrace
showing the FindItem cascade.
…rED2KPublishInfo

Belt-and-suspenders companion to the SetPublishedED2K early-return:
even after that guard skips the false→false majority, the genuinely-
true→false transitions (≤200 published per server) still pay the
O(N) FindItem cost per call. With a 100k-file shared list that's
~2×10⁷ operations on the main thread.

Add BeginBulkUpdate() / EndBulkUpdate() to CSharedFilesCtrl. While the
flag is set, UpdateItem() short-circuits; on EndBulkUpdate() we issue a
single Refresh() that repaints every visible row at once.

Wire it via the existing Notify_* dispatch (sync from main thread,
queued+FIFO from worker threads) so CSharedFileList::ClearED2KPublishInfo
can wrap its loop without leaking GUI knowledge into the daemon-only
side. AMULE_DAEMON path is a no-op as expected.
@got3nks got3nks mentioned this pull request May 9, 2026
@mifritscher2

Copy link
Copy Markdown

I can confirm that these changes do fix the disconnect GUI freeze :-)

@mrjimenez
mrjimenez merged commit 2bf9b8f into amule-project:master May 11, 2026
12 checks passed
@got3nks
got3nks deleted the fix/sharedfilesctrl-update-cascade branch May 11, 2026 08:01
got3nks added a commit to got3nks/amule that referenced this pull request Jul 23, 2026
)

The it_CH catalog was dropped in amule-project#552, but the language picker is not
driven by the shipped catalogs: it iterates the hardcoded
aMuleLanguages[] table in Preferences.cpp, which still listed
wxLANGUAGE_ITALIAN_SWISS. Removing the catalog could not hide it,
because UpdateChoice()'s probe marks an entry available when wxLocale
reports PACKAGE loaded, and wxLocale falls back it_CH -> it -- so it.mo
kept satisfying the check and the entry stayed visible.

Remove the table entry (the picker-side twin of the catalog drop) and
drop the now-unused 'Italian (Swiss)' string from the po catalogs.
Catalogs edited surgically to keep the diff to the removed entry; the
stale source-line references CI already tolerates are left untouched.
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.

100% cpu

3 participants