Noticed that we have cache writes of the AliasWhitelist cache collector implementation on every request.
The problem is that the implementation wasn't updated for the lazy-loading behavior that was added. It initializes the storage if empty, but because the storage is loaded on demand and the check is in the constructor, it does it on every single request again.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | alias-whitelist-cache-writes-2071655-3.patch | 3.25 KB | berdir |
| #3 | alias-whitelist-cache-writes-2071655-3-tests-only.patch | 2.54 KB | berdir |
| #1 | alias-whitelist-cache-writes-2071655-1.patch | 718 bytes | berdir |
Comments
Comment #1
berdirWondering how many times we already introduced stupid bugs like this (and how many of those were mine..) by trying to making something faster...
Comment #2
dawehnerI guess there is no way to generically ensure something like that, but at least test coverage for every example would work.
Comment #3
berdirMemoryCounterBackend++, poor-man-mocking :)
Comment #5
berdirFail/Pass as expected, so back to needs review.
Comment #6
catchLooks good to me.
If we had automated performance tests, we could record the number of database inserts per page and something like this would be pretty obvious. We don't have that, but this is a nice retrospective test.
Comment #7
catchNo-one complained about the RTBC, so I've gone ahead and committed/pushed this to 8.x.