Page MenuHomePhabricator

Flip Sanitizer.php attribute whitelist arrays before caching
Open, LowestPublic

Description

In a recent code review which modified Sanitizer::validateAttributes, tstarling noticed the following line, and asked an excellent question.

$whitelist = array_flip( $whitelist );

This looks slow. Why aren't they both flipped before caching?

Beats me! The $whitelist in validateAttributes is passed from an array of arrays cached as a static in setupAttributeWhitelist. validateAttributes flips $whitelist to take advantage of isset's speed over in_array. (See this blog post for discussion on isset performance.) There is only one isset statement in validateAttributes, however, so the flip would appear to undo any speed gain.

If the arrays were either flipped at the end of the initialization in setupAttributeWhitelist, or declared in flip order to begin with, that would apparently save flipping one of the arrays each time validateAttributes is called.

It would, however, change the output of public functions setupAttributeWhitelist and attributeWhitelist. Would the speed gain be worth it?

Event Timeline

MattFitzpatrick raised the priority of this task from to Lowest.
MattFitzpatrick updated the task description. (Show Details)
MattFitzpatrick subscribed.