chore: ImageMagick resource caps + server GC (MAPCO-11324) - #259
Open
shimoncohen wants to merge 2 commits into
Open
chore: ImageMagick resource caps + server GC (MAPCO-11324)#259shimoncohen wants to merge 2 commits into
shimoncohen wants to merge 2 commits into
Conversation
…APCO-11324) - ImageMagick: pin the per-operation thread count to 1 (tiles are already parallelized at the app level, so Magick's internal threading only causes oversubscription) and cap native memory at 50% of the host so a single large image cannot exhaust it. - MergerService: enable server + concurrent GC for the long-running, allocation-heavy merge workload. Scope: the hygiene ticket also lists renames / dead-code / DI dedup. The app DI registrations were checked and contain no duplicates. Broad renames and dead-code removal are deferred to avoid merge churn against the sibling MAPCO-11317 PRs (they touch the same files); this PR keeps to the self-contained resource-limit wins. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the process-global ImageMagick.ResourceLimits setup out of RegisterImageProcessors (a DI service-registration method) into an explicit ConfigureImageMagick() called once at the composition root, so the global side effect isn't hidden behind a "register services" call. Remove ResourceLimits.LimitMemory(Percentage(50)): the percentage resolves against host RAM, not the container cgroup, so it never bounded the pod (4Gi limit) and only gave false safety. Not an issue today (256x256 tiles keep the pixel cache tiny); a container-aware absolute limit is tracked as a follow-up. Thread=1 is kept — it prevents oversubscription against app-level parallelism. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
shimoncohen
requested review from
CL-SHLOMIKONCHA,
almog8k,
asafmas-rnd,
razbroc and
syncush
August 5, 2026 14:19
shimoncohen
marked this pull request as ready for review
August 5, 2026 14:19
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
1 participant
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.
LOGIC-5 of MAPCO-11317. Resource-limit hygiene.
Changes
ResourceLimits.Thread = 1. Tiles are already parallelized at the app level, so Magick's per-image OpenMP threading only oversubscribes (N app threads × M Magick threads) and adds sync overhead on 256×256 tiles. Set once at the composition root via a dedicatedConfigureImageMagick()— kept out ofRegisterImageProcessorsso this process-global side effect isn't hidden behind a DI "register services" call.ServerGarbageCollection/ConcurrentGarbageCollectionset explicitly inMergerService.csproj. These are no-ops at runtime —Microsoft.NET.Sdk.Webalready defaults both totrue— so this only pins the intent (documents it, and survives a future SDK/host change that would otherwise drop it). No behavior change.Removed
LimitMemory(50%)— dropped. The percentage resolves against host physical RAM (ImageMagick readssysconf, not the cgroup), so on the 4Gi-limited pod it resolved to ~50% of host (tens of GB) and never bounded the container — false safety; the cgroup OOM-kills at 4Gi long before it trips. Not an issue today (only 256×256 Q8 tiles flow → tiny pixel cache). A container-aware absolute limit, for if larger images are ever processed, is tracked in MAPCO-11366.Scope note
The hygiene ticket also lists renames / dead-code / DI dedup:
Program.csregistrations are distinct fromRegisterMergerLogicType).Testing
Full suite: 1141 passed, 0 failed (
Thread=1is perf-only; golden-image tests unchanged).🤖 Generated with Claude Code