Skip to content

perf(system_tags): Optimize system tag activity recipient resolution for file mapper events#59931

Open
mosi-kha wants to merge 1 commit intonextcloud:masterfrom
mosi-kha:perf/system_tags_activity
Open

perf(system_tags): Optimize system tag activity recipient resolution for file mapper events#59931
mosi-kha wants to merge 1 commit intonextcloud:masterfrom
mosi-kha:perf/system_tags_activity

Conversation

@mosi-kha
Copy link
Copy Markdown
Contributor

Summary

  • systemtags activity handling for MapperEvent::EVENT_ASSIGN / EVENT_UNASSIGN currently resolves recipients by iterating all mounts returned by getMountsForFileId() and repeatedly calling getPathsForAccessList() for the same file event.
  • On instances with many mounts, this creates significant overhead in Listener::mapperEvent() because access-list expansion is recomputed many times.
  • This change resolves a canonical file node (prefer owner-home node when available), calls getPathsForAccessList() once, and reuses that user-path map for activity publishing.

Problem

  • mapperEvent() did per-mount:
    • resolve owner folder
    • resolve node by file id
    • call getPathsForAccessList(node)
    • merge users map
  • getPathsForAccessList() already performs access-list expansion for the file node, so per-mount repetition is redundant and scales poorly with mount count.

Solution

  • Keep mount iteration only to find a resolvable node.
  • Normalize to the file owner’s home-node when available ($node->getOwner() + owner getById()), to use canonical share/access context.
  • Call getPathsForAccessList() once, assign $users = $al['users'], and stop iterating.

Why this is safe

  • Event gating is unchanged (files object type, non-empty tags, assign/unassign events, activity app enabled).
  • Activity publishing behavior is unchanged (same subject building, visibility/admin checks, and updateLastUsedTags behavior).
  • If owner-home re-resolution does not find a node, behavior falls back to the already resolved node.

Performance impact (measured example)

  • Before: user-resolution path ~34s (single event on a high-mount file).
  • After: user-resolution path ~8–12ms in the same scenario.
  • End-to-end mapper listener time dropped from tens of seconds to tens of milliseconds.

Test plan

  • Assign a tag to a file and verify activity entries for expected recipients.
  • Unassign a tag and verify corresponding activity entries.
  • Verify invisible-tag behavior remains admin-only for non-visible tags.
  • Verify behavior on shared files (including non-owner context).
  • Verify no activity is emitted when activity app is disabled.
  • Compare runtime before/after on a file with many mounts.

Notes for reviewers

  • Optimization is focused on removing repeated access-list expansion in the hot path.
  • Added inline comments explaining:
    • why owner-home node normalization is done,
    • why a single getPathsForAccessList() call is sufficient.

…ng redundant calls and improving node resolution logic
@mosi-kha mosi-kha requested a review from a team as a code owner April 27, 2026 12:51
@mosi-kha mosi-kha requested review from ArtificialOwl, icewind1991, nfebe and sorbaugh and removed request for a team April 27, 2026 12:51
@mosi-kha
Copy link
Copy Markdown
Contributor Author

mosi-kha commented May 2, 2026

@ArtificialOwl @icewind1991 @nfebe @sorbaugh
Hi guys
Can you please review my pr?

@CarlSchwan
Copy link
Copy Markdown
Member

Conflict a bit with #58602

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