ExternalConn: snapshot file lists to avoid O(N^2) GetFileByIndex loops (#666) - #687
Merged
mrjimenez merged 2 commits intoMay 23, 2026
Conversation
CSharedFileList::GetFileByIndex(i) does a std::advance() of i steps over
the underlying std::map and acquires list_mut on every call, making the
classic
for (i = 0; i < GetFileCount(); ++i) GetFileByIndex(i)
loop O(N^2) in both walk-cost and lock-acquire count. For users with
tens of thousands of shared files this pegs the main thread for tens of
minutes per EC request, blocking asio and wedging amuleweb at 100% CPU
(issue amule-project#666: 43585 shared files, ~950M iterations per refresh).
Replace the four EC hot-path loops with a single CopyFileList()
snapshot, then iterate the resulting vector outside the lock:
* Get_EC_Response_GetSharedFiles
* Get_EC_Response_GetDownloadQueue
* ECPartFileMsgSource::ECPartFileMsgSource (ctor)
* ECKnownFileMsgSource::ECKnownFileMsgSource (ctor)
The downloadqueue variants were already O(N) but re-acquired m_mutex
on every iteration; they get the same cleanup for free.
Freshness from the EC client's perspective is unchanged -- each request
already represents a single point-in-time view, the snapshot just makes
that view internally consistent (no inserts/removes smeared across the
N iterations) and reduces wall-clock from minutes to milliseconds.
CopyFileList() helpers already exist on both CSharedFileList
(SharedFileList.cpp:691) and CDownloadQueue (DownloadQueue.cpp:181).
Closed
This was referenced May 23, 2026
ngosang
added a commit
to ngosang/amule
that referenced
this pull request
Jul 30, 2026
…mule-project#687) (amule-project#719) 1bc9d6e (amule-project#694) added GET /flags/{code}.png, but nothing consumed it: the country column of the peer list and of the ed2k server list still painted only the bare uppercase code, duplicating the same one-liner cell in two views. Both now render through a single new CountryCell in components.js: the 16x11 famfamfam flag from the route followed by the code, the layout the desktop lists already use. The image URL is built relative to window.location.pathname like BASE in api.js, so it survives a reverse-proxy subpath, and the route's one-day Cache-Control means a peer list full of <img> tags doesn't re-fetch a flag per country on reload. country_code is an empty string when the daemon's GeoIP is off or the IP doesn't resolve (amule-project#439, amule-project#440), so an empty code short-circuits to the dash the cell already showed -- no <img>, and in particular no request for the "/flags/.png" the route would 404. A well-formed code the flag set has no artwork for (zz, GeoIP pseudo-codes like ap/eu) also 404s, so onError hides the image and leaves the code readable instead of a broken-image icon. The cell gains a title with the localized country name via Intl.DisplayNames ({ type: "region" }) in the UI language, as docs/api/REFERENCE.md prescribes: no endpoint and no new translation keys, since the browser already has the data. The formatter is built once per module load, not once per row. Column width goes 52px -> 70px to fit flag + code + sort arrow; sortVal is unchanged, so the column still sorts by code. Verified against a live daemon: flags render in the Networks server list and the Clients list, an empty country_code shows the dash alone, /flags/zz.png falls back to the hidden image, the tooltip follows the EN/ES switch (France/Francia), sorting works both directions, and the console stays clean.
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
2 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.
Fixes #666.
Root cause
CSharedFileList::GetFileByIndex(i)walks the shared-filestd::mapviastd::advance(begin(), i)underlist_mut. The four EC handlers that drive amuleweb iterate withwhich is O(N²) in walk cost and acquires
list_mut2N times. On @Stoatwblr's 43585-file install that is ~950M red-black-tree pointer-chases per refresh, with the main thread blocked the entire time. The asio event loop never gets to run → amuleweb sees the socket stop responding, retries, and amuled pegs a core for tens of minutes per request.Confirmed by the gdb backtrace in #666:
Fix
Replace each of the four loops with a single
CopyFileList()snapshot and iterate the resultingstd::vectoroutside the lock:Get_EC_Response_GetSharedFilesGet_EC_Response_GetDownloadQueueECPartFileMsgSourcectorECKnownFileMsgSourcectorThe two download-queue variants were already O(N) (vector backing) but re-acquired
m_mutexon every iteration; they get the same cleanup for free.Freshness
From the EC client's perspective freshness is unchanged — every request already represents a single point-in-time view. The snapshot just makes that view internally consistent (no inserts/removes smeared across N iterations) and drops the wall-clock from minutes to milliseconds. The
CopyFileList()helpers already exist on both lists (SharedFileList.cpp:691, DownloadQueue.cpp:181).Not in scope
Three other call sites use the same pattern but are O(N) only (vector backing) and outside the EC main-thread path:
BaseClient::SendSharedFilesOfDirectory, fourCDownloadQueue::Process/Save/Loadinternal loops, andCUploadQueue::Process.Test
macOS local build of
amule amuled amulegui amulewebpasses. @Stoatwblr — could you cherry-pickfix/external-conn-on2-getfilebyindex(or rebuild master with this commit applied) and confirm amuleweb stops wedging?