SharedFilesCtrl: fix O(N²) GUI cascade on server disconnect (#302) - #561
Merged
mrjimenez merged 2 commits intoMay 11, 2026
Merged
Conversation
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.
Closed
|
I can confirm that these changes do fix the disconnect GUI freeze :-) |
This was referenced May 10, 2026
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.
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.
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):
ClearED2KPublishInfo()walks every shared file and callsSetPublishedED2K(false), which unconditionally firesNotify_SharedFilesUpdateItem→CSharedFilesCtrl::UpdateItem→FindItem(-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.
SetPublishedED2Kearly-return when value didn't change (src/KnownFile.cpp). On any server-disconnect, ~99.5% of shared files are already in thefalsestate (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 sinceNotify_SharedFilesUpdateItemis a#ifndef AMULE_DAEMONGUI-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, addBeginBulkUpdate()/EndBulkUpdate(): while set,UpdateItem()short-circuits;EndBulkUpdate()issues a single fullRefresh(). Wired via the existingNotify_*dispatch (sync from main thread, FIFO-queued from workers), soCSharedFileList::ClearED2KPublishInfo(src/SharedFileList.cpp) can wrap its loop without daemon-side knowing about the GUI.Together these turn
ClearED2KPublishInfofrom O(N²) into O(N) with a single trailing repaint — for 100k files, milliseconds instead of minutes.Other
Notify_SharedFilesUpdateItemcallers were audited: the rest are single-file (rename, priority change, partfile pause/stop) or bounded-small (≤200 inOP_OFFERFILES); none enter the bulk-update window. The flag is only set duringClearED2KPublishInfo's loop, so the rest of the GUI behaves identically.Test
Smoke-tested locally on macOS aMule.app:
#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.