debugging-doctrine-performance

What this skill does

## Doctrine in a large Shopsys-based e-commerce: Developer Guide for Predictable Performance & Maintainability

This guide is meant for teams working in a Shopsys / Symfony / Doctrine codebase where application code extends vendor framework code and where business operations often touch many tables (products, prices, visibilities, translations, URLs, exports, etc.).

The goal is not to "avoid Doctrine", but to **use Doctrine in a way that stays predictable** under load and remains maintainable for humans (and AI assistants) over time.

---

# 1) Core principles (the mental model)

## Principle A — Every request/use-case should have a clear "unit of work"
Treat each controller action / CLI command as an explicit "unit of work" with:
- clear transaction boundary
- clear flush boundary
- explicit side effects (recalculations, exports, visibility refresh, URLs, cache invalidations)

If you can't explain "what happens" in a short ordered list, the code is too implicit.

---

## Principle B — "Flush ownership" must be explicit
**The #1 source of confusion is flushes happening deep in call stacks.**
It makes performance unpredictable and breaks reasoning (because a random helper can flush and trigger huge UnitOfWork work).

**Team rule (recommended):**
- Only the Facade layer is allowed to call `flush()`.
- Lower layers (Factory, Repository) must not call `flush()` unless the method name clearly states it, e.g. `saveAndFlush()` / `createAndFlush()`, and it's reviewed as intentional.

---

## Principle C — Bulk operations must have bulk code paths
Any operation that can create/update N items (variants, imports, mass edit, batch status changes) must not reuse single-item Facade methods in a loop.

Looping "rich" facade methods usually multiplies:
- DB statements
- entity hydrations
- flushes
- recalculation work

Instead: implement a bulk API or a dedicated bulk service (e.g., `*BulkCreator`).

---

## Principle D — Hot paths should be "SQL-shaped"
Even if you keep Doctrine ORM, your algorithm should feel like:
- preload everything needed
- compute changes in memory (pure logic)
- write changes in as few flushes/statements as possible
- schedule side effects once

If the code needs repeated "ask DB for a little thing" inside loops, you'll get N+1 sooner or later.

---

# 2) Do / Don't rules (general, but based on real problems)

## 2.1 Flush discipline

### Do
- **Do**: `persist()` inside loops, `flush()` once at the end.
- **Do**: if IDs are required mid-process, flush in **chunks** (e.g. 100–500), then continue.
- **Do**: design bulk operations so they can survive `clear()` if you need to control memory.

### Don't
- **Don't**: flush per entity in a loop.
- **Don't**: call framework/vendor helpers that flush implicitly from inside loops.
- **Don't**: call a "do everything" facade method repeatedly for batch creation.

**Use case hint:** If an action creates 100+ items, a per-item flush will almost always produce "Doctrine CPU spiral" symptoms.

---

## 2.2 Side effects are not free (URLs, visibilities, prices, exports, caches)

### Do
- **Do**: separate *writes* from *side effects*:
    - write core rows (products/orders/whatever)
    - then apply side effects explicitly (URLs, visibilities, exports, recalcs)
- **Do**: decide "immediate vs delayed" consciously:
    - immediate = correct right after redirect, but slower
    - delayed = fast response, eventual consistency via cron/queue

### Don't
- **Don't**: run expensive recalculation or export logic N times.
- **Don't**: rely on side effects happening "somewhere deep" in a facade chain.

**Use case hint:** Visibility recalculation and price recalculation are classic "run once per family/batch", not "per item".

---

## 2.3 Ban lazy loading in hot paths

### Do
- **Do**: use repository queries that return all data needed (join or scalar queries).
- **Do**: preload reference data into maps (IDs → data) before loops.

### Don't
- **Don't**: iterate entities and call getters that trigger lazy loads in tight loops.
- **Don't**: call repository `findOneBy()` repeatedly for the same lookup pattern.

**Use case hint:** Anything like "get translation/group/setting/price for each item" can hide N+1.

---

## 2.4 Prefer "ID-first" logic
If your loop logic only needs IDs or scalars:
- fetch scalars (DBAL/scalar DQL)
- avoid hydrating full entity graphs

This reduces both memory and UnitOfWork overhead.

---

## 2.5 Avoid "edit() as a hammer"
Large CRUD methods often do far more than "save fields".
They may trigger:
- parameter delete+insert
- image processing
- URL generation + uniqueness
- recalculation scheduling
- export scheduling
- visibility refresh

### Do
- create targeted methods for targeted changes (e.g., "set template", "link variants", "mark for export")

### Don't
- call "save everything" methods inside bulk flows or loops just to update one column.

---

# 3) Team conventions that prevent future disasters

## Convention 1 — Name methods by behavior
Examples:
- `create()` (no flush)
- `createAndFlush()` (flush inside, rare, justified)
- `createBulk()` / `bulkInsert()` (explicit bulk path)
- `scheduleRecalculations()` (explicit side effects)

This helps reviewers and AI assistants understand what happens.

---

## Convention 2 — Layering: where things belong
A maintainable structure in Shopsys typically looks like:

- **Controller**
    - input validation, form handling
- **Facade**
    - transaction boundary (via framework listener for HTTP, explicit for CLI)
    - orchestration of business logic
    - flush boundary
    - explicit side effects policy (immediate vs delayed)
- **Factory**
    - entity creation, pure logic, no DB, no flushing
- **Repository**
    - queries, persistence helpers (ideally no flush unless explicit)
- **Schedulers/Recalculators**
    - explicit calls, preferably once per use case (price, visibility, availability, export)

---

## Convention 3 — Bulk mode must be explicit
Introduce a simple policy object like:
- `BulkOperationOptions { immediateRecalc=false, immediateExport=false, chunkSize=200 }`

So code doesn't "accidentally" execute immediate work in bulk flows.

---

# 4) Practical review checklist (copy into PR template)

When reviewing a change in a "potential hot path" (mass edit/import/variant creation/list export):

1. **Flush count**
    - Is `flush()` called in loops? (Must be "no".)
    - Are there implicit flushes hidden in helpers/facades?

2. **Query count**
    - Any repository call inside loops that looks like lookup-by-id/group/translation/settings?
    - Any "per item" `findOneBy()` pattern?

3. **Lazy loading**
    - Does the loop call getters that can trigger lazy loads?

4. **Side effects**
    - Are recalculations/exports/visibility refresh happening per item?
    - Can they be done once per batch/family?

5. **Memory & UnitOfWork**
    - Are thousands of entities being created/managed unnecessarily?
    - Is `clear()` used safely (not randomly)?

---

# 5) Tooling recommendations (to enforce the rules)

- Add a debug/profiler habit for hot actions:
    - log query counts per request for selected routes
    - add "budget thresholds" (e.g., max 200 queries for admin bulk actions)
- Use Blackfire (you already do) and track:
    - flush count
    - entity count
    - query count
- Consider adding dev-only detection:
    - Doctrine SQL logger with request summary
    - fail tests if query count explodes for key actions

---

# 6) Concrete example (Option 1: keep Doctrine, make side effects explicit)

This is the pattern that works best in Shopsys-style codebases: keep Doctrine for compatibility, but force explicit orchestration.

## Example A: Command handler for creating many related items
(Keeping this example relatively close to real use cases like "create variants", but the pattern applies to imports, mass edit, bulk status updates, etc.)

### Controller (thin)
```php
public function bulkCreateAction(Request $request): Response
{
    // Validate input