Page MenuHomePhabricator

Improve Herald Rule execution speed for Personal Herald rules by avoiding expensive lookup for all adapters and instead use specific adapter check
Needs ReviewPublic

Authored by artms on Sep 12 2018, 11:15 AM.

Details

Reviewers
Pawka
epriestley
Group Reviewers
Blessed Reviewers
Summary

Adapter permission lookup is quite expensive, here we avoid looking up permissions for all adapters and instead search only one we need.

Test Plan

On empty phabricator instance (couple of users, projects) - this reduces *each* personal herald rule execution from 1.5ms to 0.3ms for Maniphest tasks.

For our production case it was reducing runtime from 20ms to 2-3ms for each Personal herald rule... We have large user base and some large projects.

This also reduces runtime for Personal Herald rules touching other objects, like differential...

Diff Detail

Repository
rP Phabricator
Branch
arcpatch-D19666
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 24968
Build 34453: Run Core Tests
Build 34452: arc lint + arc unit

Event Timeline

artms added inline comments.
src/applications/herald/adapter/HeraldAdapter.php
925

I think this is equivalent to getEnabledAdapterMap(PhabricatorUser $viewer) but without querying all adapters, hope so...

Anything? This improves herald rule performance...

See T13196 -- anything which isn't coming through a support pact may take a very long time to get to, since you're behind all open requests from paying customers.

Uh, sorry, didn't mean to step on toes, take your time :) Just wanted to share/upstream our findings which improve overall performance for large phab installations

On my personal resource limited RasberryPI alike server (4x1.2ghz ARM A53, 2GB ram, php 7.4.12) running Phabricator - this changes saves ~15ms of runtime (180ms vs 167ms after) when running:

./bin/herald test --object T19 --type HeraldManiphestTaskAdapter

It is pretty empty instance - 19 tasks, 1 user and 1 personal Herald rule...