Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
From #3578480: Worth caching allowed IP list in middleware?
I'd still suggest opening a separate issue to add a performance test. It's fairly simple, see \Drupal\Tests\redirect\FunctionalJavascript\RedirectPerformanceTest for one in contrib, or the various core subclasses which do detailed query/cache statistics. The main challenge for a contrib module is that changes in core affect the results and it could be tedious to maintain and work on all supported core versions. That's why redirect only asserts it's own queries and ban could just check a page cache hit, where changes in core are quite unlikely. You'd not want to assert assets, just number of queries/cache statistics.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork ban-3582176
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
grevil commentedWhen working on this issue, we should also check whether "IpUtils::checkIp()" has any (big) impact on performance, when used in the middleware.
Comment #4
grevil commentedOk, all done. This might help us minimally.
We could test further metrics (like CacheGetCount) , but as @berdir already said, these can change pretty fast on drupal core changes, so we only check for some basic metrics here.
Please review!
Comment #5
anybodyAs berdir wrote in the GitLab comment maybe not worth it. But we could try with a large programmatically created list of IPs. Shouldn't be a problem because we can simply use a function for generation! :)
Comment #6
anybodyI have 0 experience with performance tests, but I'm very happy to learn. For review it would be goof if @berdir could do that, due to my missing experience here.