airbnb / airbnb/MagazineLayout
Proposal: Path to removing `_enableExperimentalOptimizations`
- Dominant language
- Swift
- Stars
- 3.4k
- Forks
- 222
- PR merge metrics
- No merged PRs in 30d
Description
## Context
`MagazineLayout._enableExperimentalOptimizations` was added in #156 and documented as *"A temporary flag to enable safely testing some optimizations."* It's still default-`false` and now gates **35+ branches** across `MagazineLayout.swift`, `SectionModel.swift`, and `ModelState.swift` (PRs #158, #159, #160 each widened the surface).
I don't see a written exit plan, and wanted to put a proposal out there before more code lands behind the flag. Could be I'm missing context — happy to be told the flag is intentionally indefinite, in which case the rest of this doesn't apply.
## Why I think this matters
- Every PR behind the flag widens the divergence between the optimized and legacy paths.
- The optimized path currently has zero test coverage: `grep '_enableExperimentalOptimizations' Tests/` returns 0.
- Without parity tests + benchmarks, flipping the default is hard to do confidently.
- "Temporary" flags that live more than a couple of releases tend to become permanent forks.
## Proposed phases
**Phase 1 — Test parity** (started in #161).
Subclass the existing test classes and rerun every assertion with the flag flipped. Divergence between the optimized and legacy paths becomes a failed test. Started with `ModelStateLayoutTests`; the pattern extends mechanically to the other 4 model-state suites if you like the direction.
**Phase 2 — Benchmark harness.**
Add `MagazineLayoutBenchmarks` with XCTest `measure()` blocks for canonical scenarios:
- 1,000-item single section
- 100 sections × 10 items
- Deep self-sizing with preferred-height invalidations
- Repeated `invalidateLayout()` cycles
Capture baselines with flag=`false`, then quantify the wins with flag=`true`. Gives you numbers to back the flip and a CI gate against future regressions.
**Phase 3 — Flip the default to `true`.**
One-line change once Phases 1+2 are green. Public API surface doesn't change, but probably worth a minor version bump for visibility to consumers.
**Phase 4 — Remove the flag + delete the legacy paths.**
Cleanup PR. Same release as Phase 3 or the next minor — your call.
## Open questions
These I genuinely can't answer from outside:
1. **Is anyone using `_enableExperimentalOptimizations = true` externally?** The underscore prefix suggests SPI, but worth confirming you're not breaking a known consumer.
2. **Are there scenarios where the optimized path is intentionally divergent** from the legacy path? If yes, Phase 1 will surface them as test failures — would be good to mark them as expected up front.
3. **Release cadence** — does Airbnb's internal MagazineLayout consumer pin a version such that Phase 3 has to wait for a planned cut?
4. **Is anyone else thinking about this?** Don't want to step on an in-flight plan.
## What I'm offering
- Phase 1 is up at #161. Happy to extend the subclass pattern to the other model-state suites in a follow-up if the direction is right.
- Can take Phase 2 next — benchmark harness as a separate PR.
- Phases 3 and 4 are yours to land when you're comfortable.
If you'd rather just close this and #161 with a one-liner about why it's the wrong move, that's also a useful signal. Wanted to put the framing out there in case it's useful.
Contributor guide
Research direction
Start by reviewing _enableExperimentalOptimizations across MagazineLayout.swift, SectionModel.swift, and ModelState.swift, then inspect #161 and the existing ModelStateLayoutTests subclass pattern. Check the other four model-state suites and the proposed benchmark scenarios; done requires maintainer agreement on parity, benchmarks, external usage, and the staged removal plan.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- swift
- Domain
- mobile-dev
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100