Loading
Read authorization rows as raw pairs, not AR objects
What this does
Deciding whether someone's project access needs updating means comparing two sets: the access they should have, and the access currently recorded. Both sets were loaded as full ActiveRecord objects — one object per project the person can reach — in order to read two integers off each one.
For someone who can reach tens of thousands of projects, creating and discarding those objects costs more than everything else in the refresh put together. Most refreshes find nothing to change, so the objects are built for nothing.
Both sides now read plain [project_id, access_level] pairs instead.
Results
Measured with scripts/benchmarks/project_authorizations/, on a fixture where each person can reach 2,588 projects.
| before | after | ||
|---|---|---|---|
| Time per person | 55.4 ms | 16.3 ms | 3.4x faster |
| CPU | 1271 ms | 268 ms | −79% |
| Objects created | 1,532,569 | 747,927 | −51% |
| Garbage collections | 13 | 1 | −92% |
| Peak memory held | 4.19 MB | 3.49 MB | −17% |
| Database queries | 53 | 53 | unchanged |
| Database time | 164 ms | 170 ms | unchanged |
Database work is deliberately unchanged: the same queries run, and only the handling of their results differs. If database time had moved, something other than object creation would have changed too.
Memory measured at larger sizes, for a single set:
| Projects one person can reach | before | after |
|---|---|---|
| 2,588 | 1.2 MB | 0.1 MB |
| 10,000 | 3.8 MB | 0.4 MB |
| 46,706 | 17.8 MB | 1.8 MB |
Correctness
The benchmark records every authorization row before and after a change and fails if a single one differs. This change produces a byte-identical result across 64,700 rows, and passes the other three checks it runs: structural invariants, agreement between the two independent calculation paths, and an independent reimplementation of the rules.
Notes for review
Why the current set is read from the model rather than the association. pluck returns an association's in-memory contents when that association is already loaded, which would silently produce stale rows here. Reading from ProjectAuthorization directly avoids that. Six specs caught this during development.
Why there is no limit on the query. Database/AvoidUsingPluckWithoutLimit is disabled for one call, with the reasoning in a comment. Adding a limit would be unsafe rather than merely awkward: rows past the cut-off would never be compared, so access that should be revoked would silently persist. The set was already fully loaded before this change, using roughly ten times more memory.
Bounding it properly means not building the sets at all, which is https://gitlab.com/gitlab-org/gitlab/-/issues/625233.
Related to https://gitlab.com/gitlab-org/gitlab/-/issues/625205