Skip to content

EC: cache ed2k:// link + partmet basename on file objects for big-library performance - #725

Merged
mrjimenez merged 2 commits into
amule-project:masterfrom
got3nks:feat/ec-response-cache
May 26, 2026
Merged

EC: cache ed2k:// link + partmet basename on file objects for big-library performance#725
mrjimenez merged 2 commits into
amule-project:masterfrom
got3nks:feat/ec-response-cache

Conversation

@got3nks

@got3nks got3nks commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Two complementary caches on the daemon-side file objects to take EC response build out of the per-request hot path for large shared-file libraries. Profiling on a 91k-shared-file node (see #713) identified two CPU sinks in the CEC_SharedFile_Tag constructor that fire on every file for every EC GET_SHARED_FILES / GET_UPDATE request — both for trivially stable values:

  1. theApp->CreateED2kLink(file, …) — 42% of total daemon CPU at peak in the with-children perf trace. Recomputes the full ed2k:// URI (filename Cleanup + several CFormat substitutions) per file per call.
  2. CFormat("%s") % GetPartMetFileName().RemoveExt() for partfiles — allocates a fresh CPath (two wxStrings via DeepCopy) + CFormat round-trip per partfile per call.

Both inputs are stable across the file's lifetime under normal operation. Cache them on the file object, invalidate / never-invalidate as appropriate, and the per-request work drops to a single member access.

Commits

  1. EC: cache the ed2k:// link base on CKnownFile
    • Cache the "no sources" base form (filename + size + hash); invalidate on SetFileName (rename) — the only event that affects the link body.
    • Append the dynamic |sources,IP:port|/ suffix live when add_source is true. Tiny CFormat; mirrors CamuleAppCommon::CreateED2kLink semantics.
    • Lazy-fill on first access.
  2. EC: cache partmet basename on CPartFile too
    • One-shot cache (never invalidated — partmet basename is the allocation number, stable for life).
    • Replaces the CFormat + RemoveExt + GetPrintable round-trip in the EC tag constructor with a direct member access.

What this is and isn't

  • Build-time cache for the two heaviest per-tag fields. Eliminates the dominant ~42% CreateED2kLink CPU plus the per-partfile CFormat work.
  • Not a full-response cache: per-request iteration over every file + per-tag construction still happens. Subsequent serialization / compression / unicode2UTF8 still per-request. A separate, larger PR could add a pre-serialized FULL-response cache for amulecmd's workload if profiling after this lands still shows the build phase dominating.

Backwards compatibility

Wire format unchanged. EC clients receive identical EC_TAG_PARTFILE_ED2K_LINK / EC_TAG_KNOWNFILE_FILENAME payloads. Any client version (old or new) interoperates with any amuled version (old or new) — the cache is purely an internal data structure on the daemon side.

Memory cost

  • 1 wxString per CKnownFile (the ed2k link base) — populated on first request, ~50–200 bytes per shared file.
  • 1 additional wxString per CPartFile (the partmet basename) — populated once after the partfile is created.

For a 91k-shared-file node: ~10–20 MB additional steady-state heap. Tiny relative to the existing per-file metadata.

Test plan

  • macOS Debug build of amuled, amulecmd, amulegui — clean compile
  • Smoke test: amuled started, EC show shared, show DL, Shutdown via amulecmd against an empty shareset — no crash, both cache paths exercised
  • Profiled re-run by @Stoatwblr on his 91k-file setup to measure the actual build-time delta (the dominant CreateED2kLink entry should drop from the perf top)

Refs #713.

got3nks added 2 commits May 26, 2026 12:29
Building the ed2k:// link via theApp->CreateED2kLink was the dominant
CPU cost in EC response dispatch for large shared-file libraries — 42%
of total daemon CPU on a 91k-shared-file profile (issue amule-project#713). Every
EC GET_SHARED_FILES (FULL detail, what amulecmd issues per invocation)
and every EC GET_UPDATE (INC_UPDATE detail, what amulegui issues
per cycle) reconstructs the link for every file in the shareset, even
when the file's exported state hasn't changed.

The link body depends on filename, size and hash — all three are
stable across the lifetime of a CKnownFile / CPartFile under normal
operation. The only event that affects them is SetFileName (rename).
Cache the base form ("no sources" variant) on the file object,
invalidate it on SetFileName, lazy-fill on first access.

The optional |sources,IP:port|/ suffix depends on dynamic state
(IsConnected, IsFirewalled, GetID, GetPort) that the cache cannot
hold; append it live when add_source is true. The CFormat for the
suffix is much cheaper than the full link build it replaces (one
small CFormat vs filename Cleanup + several CFormat substitutions
for the file portion).

Wire format unchanged — EC clients receive identical
EC_TAG_PARTFILE_ED2K_LINK payloads. Backwards-compatible with any
amulegui/web/cmd version, old or new.

Refs amule-project#713.
Companion to the ed2k:// link cache. The CEC_SharedFile_Tag constructor's
EC_TAG_KNOWNFILE_FILENAME branch for partfiles used to go through
`CFormat("%s") % GetPartMetFileName().RemoveExt()` on every call — that
allocates a fresh CPath (two wxStrings via DeepCopy) plus a CFormat
round-trip per file per EC cycle.

The partmet filename is set when the partfile is created and never
changes for its lifetime, so this is a populate-once cache with no
invalidation surface. Lazy-fill on first access, cache forever.

Refs amule-project#713.
@got3nks
got3nks marked this pull request as ready for review May 26, 2026 22:12
@mrjimenez
mrjimenez merged commit 35e8f1c into amule-project:master May 26, 2026
7 checks passed
@got3nks
got3nks deleted the feat/ec-response-cache branch May 27, 2026 15:15
got3nks pushed a commit to got3nks/amule that referenced this pull request Jul 31, 2026
…r.cpp (amule-project#675) (amule-project#725)

* chore(gui): delete unreachable bitmap functions/entries from muuli_wdr.cpp

First slice of the icon-system cleanup scoped in amule-project#675: remove code with
zero call sites anywhere in the tree, before any wxArtProvider migration
work starts.

- muleToolbar(): whole function unused -- superseded by the main
  wxToolBar setup in amuleDlg.cpp; nothing calls it.
- moreImages(): whole function unused, both of its two icon entries.
- amuleDlgImages(): 21 of 35 index blocks have no caller anywhere
  (0-13, 16, 17, 19, 27, 28, 31, 34). The 14 live ones are untouched --
  10 of those (20-26, 29, 32, 33) are already the fallback path inside
  amuleDlg.cpp's Add_Skin_Icon, which prefers a wxArtProvider/SVG lookup
  first; the other 4 (14, 15, 18, 30) are still called directly.
- amuleSpecial(): 6 of 26 index blocks have no caller (6, 7, 8, 9, 18,
  20) -- checked both direct call sites and the PrefsUnifiedDlg.cpp
  fallback table (pages[].m_imageidx), which uses neither.

convert_xpm in PartFileConvertDlg.cpp was on the same "no literal
grep hits" list initially but is not actually dead -- SetIcon(wxICON
(convert)) reaches it via the wxICON macro's token-pasting
(X##_xpm), invisible to a plain identifier search. Caught by a full
build failing on the undeclared identifier, not by inspection; left
untouched.

Deletes 1373 lines (~16% of the file). No behavior change: every
touched entry was unreachable code. clang-format v18 clean; full
amule build verified (macOS, CLIENT_GUI unaffected since neither
touched symbol is CLIENT_GUI-only).

* chore(po): regenerate catalogs after muuli_wdr.cpp dead-code removal

muleToolbar() duplicated several msgid source-location references
(Networks, Searches, Downloads Window, etc.) already present via
amuleDlg.cpp's own toolbar setup. Deleting it drops those now-stale
#: comments; no msgid added, removed, or retranslated -- verified via
unchanged msgid count (1903 before/after) and a diff limited to
source-location comments and POT-Creation-Date.

Regenerated after rebasing onto upstream/master to pick up po/
changes from amule-project#723/amule-project#724/amule-project#726/amule-project#728/amule-project#730/amule-project#731, which had drifted our
prior regeneration out of sync.
mrjimenez pushed a commit to mrjimenez/amule that referenced this pull request Aug 1, 2026
… (amule-project#739)

* feat(gui): wxFAIL_MSG on unexpected icon-bank index (amule-project#675)

got3nks (PR amule-project#725 review): every raw-bitmap bank in this file
(clientImages, amuleSpecial, connButImg, amuleDlgImages) fails
silently -- an index with no matching block falls through to
wxNullBitmap, i.e. a blank icon, with no compiler error, no link
error, and nothing for CI to catch. That's exactly how an incorrectly
deleted amuleSpecial(25) almost shipped in amule-project#725: it would have
compiled clean in every configuration and only shown up to a human
who happened to open the right dialog in a debug build.

wxFAIL_MSG matches this codebase's existing assert idiom (see
GenericClientListCtrl.cpp, MuleListCtrl.cpp) and is a no-op outside
debug builds, so this is a debug-only safety net with no release
behaviour change. Doing it now, ahead of the riskier remaining slices,
matters most for clientImages: it's consumed with index arithmetic
across four call sites (Client_InvalidRating_Smiley + rating - 1), so
an off-by-one there is exactly this failure mode -- one blank smiley,
nothing failing anywhere. The assert turns that into an immediate stop
on the first debug run instead of a maybe-someday-noticed bug.

Verified via a full Debug-config build (macOS, wxDEBUG_LEVEL default,
not the Win32-only wxDEBUG_LEVEL=0 override) and a visual pass through
every tab/dialog that reaches one of these banks (Reti, Messaggi, File
condivisi, Preferenze) -- no assertion fired, confirming every index
currently in use is legitimate.

* docs(gui): note amuleSpecial(25)'s __DEBUG__-only consumer

PR amule-project#725 review: this index looked dead by the same criterion used for
the ones actually removed, but it's reachable via
PrefsUnifiedDlg.cpp's pages[] table -- one entry ("Debugging",
PreferencesDebug) is guarded by #ifdef __DEBUG__ and breaks the
Preferences<Something>Tab naming convention every other entry follows,
which is exactly what made it easy to miss with a naming-pattern-based
grep or a release-only build. Deleting it wouldn't have failed any
build (falls through to wxNullBitmap), so the only symptom would have
been a blank icon on a debug-only preferences page -- worth spelling
out for the next pass over this file.

---------

Co-authored-by: got3nks <[email protected]>
mrjimenez pushed a commit to mrjimenez/amule that referenced this pull request Aug 1, 2026
…or tab-close (amule-project#675) (amule-project#735)

* feat(gui): migrate Friends/Messages headers, reload buttons, folder tree, and tab-close icons to CamuleArtProvider

Continues the icon-system cleanup scoped in amule-project#675, independent of
amule-project#732/amule-project#733 (different files, no overlap).

- amuleDlgImages(14)/(15): the small header icons in the Friends and
  Messages tabs. Friends gets a new "amule:friends" SVG; Messages
  reuses the existing "amule:toolbar_messages" art, requested at an
  explicit 16x16 so it doesn't inherit the toolbar's 32x32 natural
  size.
- amuleDlgImages(18)/(30): the "reload list" buttons (shared files,
  ED2K server list, Kad node list) -- three call sites, one new
  "amule:reload" SVG (see below for its own history).
- amuleSpecial(1)/(2): the shared-directory tree's folder icons get
  new "amule:folder"/"amule:folder_shared" SVGs -- same shape, tinted
  orange vs. red, matching how the original raw bitmaps only differed
  by colour.
- amuleSpecial(3)/(4): the chat/search notebook tabs' close-on-hover
  icon, replaced with wx's own stock wxART_CLOSE for both states
  instead of a second bespoke asset (both were the same "X in a box"
  bitmap, differing only by a hover-highlight border colour).

reload.svg's own history, per PR review: the first hand-drawn attempt
(thin blue single arc) didn't match the original at all -- got3nks
caught it, I reconstructed the original amuleDlgImages(18) raster to
check (a thicker green double-arrow circle) and redrew closer to that,
which still wasn't good enough. Final version is AI-vectorized via
Recraft (through the Higgsfield MCP), which got3nks preferred over
further hand-drawn iterations.

src/icons/icon_data.c (the checked-in fallback used when Python3 is
absent at configure time, per amule-project#487) regenerated via embed_icons.py to
match.

Rebased onto current master (picking up amule-project#732/amule-project#733/amule-project#739, which all
touch the same amuleSpecial/amuleDlgImages functions) -- conflicts
resolved by redoing the index deletions against the current tree
rather than replaying the stale patch, since amule-project#725/amule-project#733 already moved
the surrounding line numbers.

Verified via a full amule build (macOS) and a visual check of every
call site: Friends/Messages tab headers, the ED2K server-list and Kad
node-list reload buttons, the shared-files reload button, and the
Preferences > Directory shared-folder tree.

* fix(gui): thicker reload stroke, dedicated message-bubble icon (amule-project#735 review)

got3nks, testing amule-project#735:
- reload: arrow bodies read too thin at 16px. Regenerated via Recraft
  with an explicit thicker/bolder-stroke prompt.
- Messages panel header: reusing "amule:toolbar_messages" (the main
  toolbar's detailed gradient mascot bust) at 16x16 doesn't read as
  "messages" once shrunk that far from its 32x32 native size. Added a
  dedicated "amule:message" chat-bubble glyph instead, sized for
  legibility at 16x16 specifically, and pointed the Messages panel
  header at it instead of the toolbar art.

Folder/folder_shared and the tab-close X were already approved as-is.

Verified via a full amule build (macOS) and a visual check of both
fixes: the shared-files reload button and the Messages panel header.
mrjimenez pushed a commit to mrjimenez/amule that referenced this pull request Aug 4, 2026
…rovider (amule-project#675) (amule-project#747)

* feat(gui): migrate 22 of 27 clientImages() status icons to CamuleArtProvider (amule-project#675)

Continues the icon-system cleanup scoped in amule-project#675. clientImages() is
the last of the raw-bitmap banks, and the riskiest: it's consumed
through a wxImageList built once at startup (Apply_Clients_Skin,
amuleDlg.cpp) by iterating index 0..26 in ClientSkinEnum order, and
four call sites then index that list with arithmetic
(Client_InvalidRating_Smiley + rating - 1) rather than calling
clientImages() directly. The enum order is load-bearing; this PR does
not touch it, the calling loop, or the arithmetic call sites -- only
what clientImages(index) returns for each index.

22 of the 27 icons are generic pictograms (smileys, a checkmark, an
X, stars, an arrow, a key, ...) with no external branding, migrated to
new "amule:client_<name>" SVG/PNG art. The remaining 5 -- indices 12,
15, 16, 17, 18 (eMule/aMule/lphant/Shareaza/xMule) -- are specific
client-software mascots and are deliberately left as the original raw
artwork: a generated pictogram is one thing, but redrawing someone
else's brand mascot risks not matching the real logo, which is worse
than not touching it. amule-project#675 discussion flagged this distinction
explicitly.

New art, per icon:
- Green/Red/Yellow/Grey/White: the 5 base transfer-state smileys.
- ExtendedProtocol/SecIdent/BadGuy/Encryption: badge overlays drawn
  per GenericClientListCtrl.cpp's own comments ("the '-'", "the 'v'",
  "the 'X'", "the '\xc2\xbf' except it's a key").
- CreditsGrey/CreditsYellow: the two credit-system stars.
- Upload: the upload-active arrow.
- Friend: the friend-list badge.
- mlDonkey/eDonkeyHybrid: kept generic/abstract (a checkmark badge, a
  simple ghost) rather than attempting their real logos -- same
  reasoning as the 5 mascots, lower bar since these were already more
  abstract marks than full character art in the original raster.
- Unknown: the unresolved-client-type question mark.
- InvalidRating/PoorRating/FairRating/GoodRating/ExcellentRating: one
  potion-bottle SVG re-tinted per rating (red/red/orange/green/green),
  with Invalid and Excellent composited as two bottles side by side in
  a single 16x16 square canvas -- matching the original art's
  single-vs-double-bottle distinction for the two rating extremes.
- CommentOnly: the comment-icon badge.

Implementation: a static art-id lookup table indexed by ClientSkinEnum
value, checked first; a null entry falls through to the corresponding
raw-bitmap `if (index == N)` block for the 5 mascot indices, which are
otherwise untouched. src/icons/icon_data.c regenerated via
embed_icons.py to match.

Testing: full build verified (macOS), both Release and Debug config.
Debug build launched with a live window (Apply_Clients_Skin runs at
startup, touching all 27 indices) and produced no wxFAIL_MSG assertion
(amule-project#739) -- confirms every index resolves. Each new icon reviewed
individually via rasterized contact sheets at both ~96px and actual
16x16 before being wired in.

NOT independently verified: seeing these icons rendered in an actual
populated client list (Downloads sources, Shared Files clients, Search
results, Friends) requires live eD2k/Kad network data, which a
from-scratch test config doesn't have. Same limitation already flagged
for fileDetails/clientDetails/CategoriesEditWindow in amule-project#744.

* fix(gui): recentre double-bottle rating icons, add ClientSkinEnum static_assert (amule-project#747 review)

got3nks, reviewing amule-project#747:

1. client_invalidrating.svg / client_excellentrating.svg didn't fill
   their 2048x2048 viewport like their poor/fair/good siblings (53%x73%
   fill for those vs. 83%x44% for these, with the left bottle clipped
   at x=-47). Recentred both bottle <g> transforms to
   translate(-110,452)/translate(940,452) with scale raised to 0.8
   (from 0.6), matching the visual weight of the single-bottle ratings
   next to them in the same column. Both files are otherwise identical
   apart from fill colour, so the one fix applies to both.

2. Nothing tied artIds[] to ClientSkinEnum, and wxFAIL_MSG (amule-project#739)
   compiles out in release builds -- so a future enum member could
   silently push a wxNullBitmap into the 16x16 image list instead of
   failing loudly anywhere. Added
   static_assert(WXSIZEOF(artIds) == CLIENT_SKIN_SIZE, ...), which
   catches it at compile time in every build config. Needed
   "amuleDlg.h" (CLIENT_SKIN_SIZE's home) added to muuli_wdr.cpp's
   includes -- checked first that amuleDlg.h doesn't include
   muuli_wdr.h back, no circular include.

Verified via a full amule build (macOS) -- the static_assert compiling
clean confirms artIds[] and ClientSkinEnum agree on 27 entries, and a
visual check that the two rating icons now match their siblings' size
and no longer clip.

* fix(gui): recompute double-bottle composition, verified numerically (amule-project#747 review)

My previous fix (eedcac3) just moved the problem: applying got3nks's
suggested translate values verbatim together with the also-suggested
scale=0.8 pushed the right bottle past the viewBox edge (measured:
x range 13%-107%, i.e. clipped on the right instead of the left).
The two numbers were each individually reasonable but hadn't been
re-verified together -- exactly the mistake the last commit's message
claimed to have checked and hadn't.

Recomputed from the natural bounding box instead of iterating on
coordinates by eye: two bottles at scale 0.68 with a 40-unit gap, each
positioned so both are fully inside the 0-2048 viewBox with equal
margins. Verified numerically (not just visually) before committing:
both groups' bboxes land at x 12.9%-49.0% and 51.0%-87.1%, y
25.2%-74.8% -- fully in bounds, no clipping on either side.

Worth being upfront about the height: overall fill is 74%x50%, not
poor/fair/good's 53%x73%. That's not a leftover bug -- fitting two
full-height copies of the same bottle side by side in a square without
overlap is not geometrically possible; matching the single bottles'
73% height would require the pair to be over twice as wide as the
viewBox. 50% is the tradeoff of choosing "fits, no clipping, similar
per-bottle proportions" over an unreachable exact size match.

Verified via a full amule build (macOS) and the bounding-box
computation above, re-run against the actual committed SVG files
rather than the standalone drafts, to catch exactly the kind of
last-mile mismatch that slipped through last time.

* fix(gui): unify bottle height across all 5 rating icons (amule-project#747 review)

got3nks: rather than treat the double-bottle pair as a special case
with its own (necessarily shorter) height, make all five ratings
share one consistent bottle height, maximizing what the double-bottle
pair allows without clipping and bringing poor/fair/good down to
match -- they sit in the same column, so consistency across the set
matters more than any one icon being as large as it could be alone.

Recomputed from the natural bottle bounding box: scale 0.909 is the
largest that fits two bottles side by side (24-unit gap) inside the
2048x2048 viewBox with no clipping. Applied that same scale to the
single-bottle ratings, centred. All five now render their bottle at
the same height.

Verified numerically against the actual committed files: all five
land at y 16.8%-83.2% (66.3% height) and are within the 0-100% x
range -- identical height across poor/fair/good/invalid/excellent,
zero clipping.

Verified via a full amule build (macOS).

* fix(gui): brighten client_green to match the rest of the set's saturation (amule-project#747 review)

got3nks measured across every opaque pixel: red and yellow gained
brightness/saturation moving to the new art, but green lost both,
landing dimmer (53% mean brightness) than red (64%) and yellow (72%)
-- inverting the original's balance, where green and red were level
at 57%/55%. Green carries the most meaning of the three (actively
transferring), so it being the dimmest state undersells it.

Replaced Material Green 500 (#4CAF50, 57% sat / 69% val) with
#22CC1A (87% sat / 80% val), inside the ~#1BD816-#2ECC1F range
got3nks suggested to restore the original's emphasis. Red and yellow
untouched, per the review ("read fine as they are").

Verified via a full amule build (macOS) and a visual check of the
icon at 96px.

* fix(gui): corner-anchor badge overlays, add null-bitmap guard (amule-project#747 review)

got3nks caught a real functional regression I'd missed (a second,
earlier review comment I hadn't seen when I replied to the later one
about colour): 6 of these icons aren't standalone -- GenericClientListCtrl.cpp
draws them as overlays on top of the base client icon at the same
point.x/realY. The original raw art kept each in a corner so the base
icon stayed visible underneath; my replacements were designed as
standalone centered pictograms, so they covered the icon they were
meant to annotate (creditsyellow at 14x13 was larger than the 12x12
base smiley it sat on).

Recomputed each as a corner badge, scaled/positioned to roughly the
original's measured footprint and anchor (his measurements, converted
from 16x16 pixel terms to the 2048x2048 viewBox):
- extendedprotocol: top-right, ~8x4
- secident: bottom-left, ~7x6
- creditsgrey/creditsyellow: top-right, ~9x9
- encryption: top-left, ~6x8

badguy is the one exception, and deliberately so: the original struck
through the *entire* icon rather than badging a corner, which reads as
intentional (a bad-guy client's status is fully overridden, not just
annotated) -- kept that full-cover behaviour rather than shrinking it
to a corner mark.

Also added the null-bitmap guard he flagged: wxArtProvider::GetBitmap()
returning wxNullBitmap for an unresolvable id was returning straight
through the new early-return path, never reaching the wxFAIL_MSG that
amule-project#739 added for exactly this failure mode. Added a wxASSERT_MSG on the
early-return path so a future icon rename can't silently regress back
to the "fails into a blank icon" state amule-project#725/amule-project#739 fixed.

Verified via a full amule build (macOS) and a visual check of all 6
badges at both 96px and actual 16x16, confirmed each sits in its
corner without covering the centre of the base icon.
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.

2 participants