Page MenuHomePhorge

PhabricatorEnv: add domains cache for isSelfURI()
Needs ReviewPublic

Authored by valerio.bozzolan on Tue, May 6, 16:34.

Details

Summary

Premising that the constructor of PhutilURI is relatively slow,
because it calls various regular expressions to match the domain, the anchor, etc.

https://we.phorge.it/source/arcanist/browse/master/src/parser/PhutilURI.php

Note that the method PhabricatorEnv::isSelfURI() creates multiple PhutilURI objects,
just to check if the configuration domains are matched.

Note that isSelfURI() can be called multiple times during a page lifecycle,
especially for big Remarkup documents with many internal/external links.

So, we increase the efficiency of PhabricatorEnv::isSelfURI(), by adding a minimal domains
cache, so to avoid the re-creation of PhutilURI objects for each isSelfURI() call.

This should be safe since the global configuration is not supposed to change during the page load.
Anyway, we already covered the already-existing dropConfigCache() that is called when this happens,
for example in some unit tests.

Good for your CPU, for your performances, for your electric bills, for the stress of your
your garbage collector, and for your environment. You're welcome Greta Thunberg.

Closes T16061

Test Plan
arc unit ./src/infrastructure/parser/__tests__/PhutilPygmentizeParserTestCase.php
arc unit ./src/infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php
arc unit ./src/infrastructure/parser/__tests__/PhutilURIHelperTestCase.php
arc unit ./src/infrastructure/markup/__tests__/PhutilTranslatedHTMLTestCase.php

Additionally, manually craft the new cache to phlog('CACHE BUILT') in one case or phlog('CACHE HIT') in the other,
and see that the cache is clearly in place doing its job, showing only CACHE BUILT the first time, and then CACHE HIT.

Diff Detail

Repository
rP Phorge
Branch
asd-bis
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1937
Build 1937: arc lint + arc unit

Event Timeline

valerio.bozzolan held this revision as a draft.
valerio.bozzolan retitled this revision from PhabricatorEnv: add domains cache for isSelfURI() Premising that the constructor of PhutilURI is relatively slow, because it calls various regular expressions to match the domain, the anchor, etc. Note that the method PhabricatorEnv::isSelfURI()... to PhabricatorEnv: add domains cache for isSelfURI(.Tue, May 6, 16:35
valerio.bozzolan edited the summary of this revision. (Show Details)
valerio.bozzolan retitled this revision from PhabricatorEnv: add domains cache for isSelfURI( to PhabricatorEnv: add domains cache for isSelfURI().
valerio.bozzolan edited the summary of this revision. (Show Details)

Ready for review :3 based on the other one.

src/infrastructure/env/PhabricatorEnv.php
462

✅ I've decided to maintain this private method as-is, so to 100% keep its origin git blame