Cutting Per-Request Work in Horilla's Breadcrumbs Context Processor

A Django performance fix, sharpened by one precise review request.

Kort antwoord

Horilla's breadcrumbs() context processor runs on every rendered page. It loaded every Employee row, and every active Candidate row, as full model objects just to read their IDs, then wrote the list to the session on almost every request. The fix fetches IDs with values_list, builds querysets only in the branch that needs them, and writes to the session only when the list changes. It was merged as PR #1180 and closed issue #1038.

The problem

Horilla is an open-source HR management system built with Django. Issue #1038 reported high latency in a Locust load test, with some requests taking "even 40 secs".

Root cause

The maintainer summarized it in review: "breadcrumbs() is a context processor, so it runs on every rendered request, and before this change it unconditionally evaluated Employee.objects.all() and Candidate.objects.filter(is_active=True) ... then materialised every row into model instances just to read .id." They added a point the PR description had only touched on: "the old code assigned to request.session[...] on every request, which marks the session dirty and forces a session write on every page load."

The fix

def sync_session_ids(request, key, queryset):
    ids = list(queryset.order_by("id").values_list("id", flat=True))
    if request.session.get(key) != ids:
        request.session[key] = ids

The callers now build querysets only inside the branch that uses them, and the recruitment branch checks apps.is_installed("recruitment") before importing Candidate.

By the numbers

• Repository: horilla/horilla-hr, 1,486 stars (Nov 3, 2026), Python (GitHub classifies the repo as HTML), LGPL-2.1
• Files changed: 1
• Lines: +15 / -19
• Commits: 2
• Opened: Aug 21, 2026. Merged: Sep 9, 2026. Time to merge: 18 days, 21 hours
• Review: one change requested, then approved and merged by the maintainer team

How the review went

The maintainer agreed with the diagnosis ("This is a good catch and the diagnosis is right") and asked for one change. The new comparison depends on a stable order, but Employee is ordered by first name and Candidate by sequence, neither of which is unique:

Adding .order_by("id") inside sync_session_ids makes the comparison deterministic regardless of what the caller passes

The follow-up commit did exactly that. The maintainer's reply: "that's exactly it.... Nothing further from me. Mergeable, and this is the one in the queue with a measurable effect on every page load."

They also noted that CI doesn't run on fork PRs in the repo, calling it "our gap, not yours" and adding "we verify these by hand."

Takeaways

• Context processors are hot paths. Anything they do, every page pays for.
• Writing to the session only on change is as important as the cheaper query.
• The review caught a correctness detail, the ordering, that would have quietly weakened the optimization.

Veelgestelde vragen

Did the fix add tests?

No test file is included in the merged diff.

Where can I read more?

See the pull request and the original issue.

Het doel

Performance fixes earn trust when the reasoning is visible. Here the maintainer could check every claim against the code, and asked for the one change that made the comparison hold.