[3.1.1] Memory optimizations for ConfigSchema. Changes include:

- Elimination of ConfigDef and subclasses in favor of stdclass. Most property names stay the same - Added benchmark script for ConfigSchema - Types are internally handled as magic integers. Use HTMLPurifier_VarParser->getTypeName to convert to human readable form. HTMLPurifier_VarParser still accepts strings. - Parser in config schema only used for legacy interface git-svn-id: http://htmlpurifier.org/svnroot/htmlpurifier/trunk@1764 48356398-32a2-884e-a903-53898d9a118a
fdicioccio · May 23, 2008 · 8ab30e2 · 8ab30e2
1 parent 9db891c
commit 8ab30e2
Show file tree

Hide file tree

Showing 18 changed files with 171 additions and 218 deletions.
diff --git a/NEWS b/NEWS
@@ -31,6 +31,12 @@ NEWS ( CHANGELOG and HISTORY )                                     HTMLPurifier
   will still work--the new format, however, lets modules access the
   configuration object for HTML namespace dependant tweaks.
 . AttrDef_HTML_Pixels now takes a single construction parameter, pixels.
+. ConfigSchema data-structure heavily optimized; on average it uses half
+  the memory it did previously. The interface has changed accordingly,
+  consult changes to HTMLPurifier_Config for details.
+. Variable parsing types now are magic integers instead of strings
+. Added benchmark for ConfigSchema
+
 
 3.1.0, released 2008-05-18
 # Unnecessary references to objects (vestiges of PHP4) removed from method

diff --git a/TODO b/TODO
@@ -11,8 +11,6 @@ If no interest is expressed for a feature that may require a considerable
 amount of effort to implement, it may get endlessly delayed. Do not be
 afraid to cast your vote for the next feature to be implemented!
 
-- Ability to fully turn off imagecrash fixes (attribute and CSS will require
-  two separate directives due to our architecture.)
 - Investigate how early internal structures can be accessed; this would
   prevent structures from being parsed and serialized multiple times.
 - Figure out how to simultaneously set %CSS.Trusted and %HTML.Trusted (?)

diff --git a/benchmarks/ConfigSchema.php b/benchmarks/ConfigSchema.php
@@ -1,5 +1,9 @@
 <?php
 
+chdir(dirname(__FILE__));
+
+//require_once '../library/HTMLPurifier.path.php';
+shell_exec('php ../maintenance/generate-schema-cache.php');
 require_once '../library/HTMLPurifier.path.php';
 require_once 'HTMLPurifier.includes.php';
 

diff --git a/library/HTMLPurifier.includes.php b/library/HTMLPurifier.includes.php
@@ -29,7 +29,6 @@
 require 'HTMLPurifier/CSSDefinition.php';
 require 'HTMLPurifier/ChildDef.php';
 require 'HTMLPurifier/Config.php';
-require 'HTMLPurifier/ConfigDef.php';
 require 'HTMLPurifier/ConfigSchema.php';
 require 'HTMLPurifier/ContentSets.php';
 require 'HTMLPurifier/Context.php';
@@ -127,9 +126,6 @@
 require 'HTMLPurifier/ChildDef/Optional.php';
 require 'HTMLPurifier/ChildDef/StrictBlockquote.php';
 require 'HTMLPurifier/ChildDef/Table.php';
-require 'HTMLPurifier/ConfigDef/Directive.php';
-require 'HTMLPurifier/ConfigDef/DirectiveAlias.php';
-require 'HTMLPurifier/ConfigDef/Namespace.php';
 require 'HTMLPurifier/DefinitionCache/Decorator.php';
 require 'HTMLPurifier/DefinitionCache/Null.php';
 require 'HTMLPurifier/DefinitionCache/Serializer.php';

diff --git a/library/HTMLPurifier.safe-includes.php b/library/HTMLPurifier.safe-includes.php
@@ -23,7 +23,6 @@
 require_once $__dir . '/HTMLPurifier/CSSDefinition.php';
 require_once $__dir . '/HTMLPurifier/ChildDef.php';
 require_once $__dir . '/HTMLPurifier/Config.php';
-require_once $__dir . '/HTMLPurifier/ConfigDef.php';
 require_once $__dir . '/HTMLPurifier/ConfigSchema.php';
 require_once $__dir . '/HTMLPurifier/ContentSets.php';
 require_once $__dir . '/HTMLPurifier/Context.php';
@@ -121,9 +120,6 @@
 require_once $__dir . '/HTMLPurifier/ChildDef/Optional.php';
 require_once $__dir . '/HTMLPurifier/ChildDef/StrictBlockquote.php';
 require_once $__dir . '/HTMLPurifier/ChildDef/Table.php';
-require_once $__dir . '/HTMLPurifier/ConfigDef/Directive.php';
-require_once $__dir . '/HTMLPurifier/ConfigDef/DirectiveAlias.php';
-require_once $__dir . '/HTMLPurifier/ConfigDef/Namespace.php';
 require_once $__dir . '/HTMLPurifier/DefinitionCache/Decorator.php';
 require_once $__dir . '/HTMLPurifier/DefinitionCache/Null.php';
 require_once $__dir . '/HTMLPurifier/DefinitionCache/Serializer.php';

diff --git a/library/HTMLPurifier/Config.php b/library/HTMLPurifier/Config.php
@@ -125,7 +125,7 @@ public function get($namespace, $key) {
                 E_USER_WARNING);
             return;
         }
-        if ($this->def->info[$namespace][$key]->class == 'alias') {
+        if (isset($this->def->info[$namespace][$key]->isAlias)) {
             $d = $this->def->info[$namespace][$key];
             trigger_error('Cannot get value from aliased directive, use real name ' . $d->namespace . '.' . $d->name,
                 E_USER_ERROR);
@@ -196,7 +196,7 @@ public function set($namespace, $key, $value, $from_alias = false) {
                 E_USER_WARNING);
             return;
         }
-        if ($this->def->info[$namespace][$key]->class == 'alias') {
+        if (isset($this->def->info[$namespace][$key]->isAlias)) {
             if ($from_alias) {
                 trigger_error('Double-aliases not allowed, please fix '.
                     'ConfigSchema bug with' . "$namespace.$key", E_USER_ERROR);
@@ -212,20 +212,20 @@ public function set($namespace, $key, $value, $from_alias = false) {
             $value = $this->parser->parse(
                         $value,
                         $type = $this->def->info[$namespace][$key]->type,
-                        $this->def->info[$namespace][$key]->allow_null
+                        isset($this->def->info[$namespace][$key]->allow_null)
                      );
         } catch (HTMLPurifier_VarParserException $e) {
-            trigger_error('Value for ' . "$namespace.$key" . ' is of invalid type, should be ' . $type, E_USER_WARNING);
+            trigger_error('Value for ' . "$namespace.$key" . ' is of invalid type, should be ' . HTMLPurifier_VarParser::getTypeName($type), E_USER_WARNING);
             return;
         }
         if (is_string($value)) {
             // resolve value alias if defined
             if (isset($this->def->info[$namespace][$key]->aliases[$value])) {
                 $value = $this->def->info[$namespace][$key]->aliases[$value];
             }
-            if ($this->def->info[$namespace][$key]->allowed !== true) {
+            if (isset($this->def->info[$namespace][$key])) {
                 // check to see if the value is allowed
-                if (!isset($this->def->info[$namespace][$key]->allowed[$value])) {
+                if (isset($this->def->info[$namespace][$key]->allowed) && !isset($this->def->info[$namespace][$key]->allowed[$value])) {
                     trigger_error('Value not supported, valid values are: ' .
                         $this->_listify($this->def->info[$namespace][$key]->allowed), E_USER_WARNING);
                     return;
@@ -386,7 +386,7 @@ public static function getAllowedDirectivesForForm($allowed, $schema = null) {
                     if (isset($blacklisted_directives["$ns.$directive"])) continue;
                     if (!isset($allowed_directives["$ns.$directive"]) && !isset($allowed_ns[$ns])) continue;
                 }
-                if ($def->class == 'alias') continue;
+                if (isset($def->isAlias)) continue;
                 if ($directive == 'DefinitionID' || $directive == 'DefinitionRev') continue;
                 $ret[] = array($ns, $directive);
             }

diff --git a/library/HTMLPurifier/ConfigDef.php b/library/HTMLPurifier/ConfigDef.php
diff --git a/library/HTMLPurifier/ConfigDef/Directive.php b/library/HTMLPurifier/ConfigDef/Directive.php
diff --git a/library/HTMLPurifier/ConfigDef/DirectiveAlias.php b/library/HTMLPurifier/ConfigDef/DirectiveAlias.php
diff --git a/library/HTMLPurifier/ConfigDef/Namespace.php b/library/HTMLPurifier/ConfigDef/Namespace.php
diff --git a/library/HTMLPurifier/ConfigSchema.php b/library/HTMLPurifier/ConfigSchema.php
@@ -12,7 +12,28 @@ class HTMLPurifier_ConfigSchema {
     public $defaults = array();
 
     /**
-     * Definition of the directives.
+     * Definition of the directives. The structure of this is:
+     * 
+     *  array(
+     *      'Namespace' => array(
+     *          'Directive' => new stdclass(),
+     *      )
+     *  )
+     * 
+     * The stdclass may have the following properties:
+     * 
+     *  - If isAlias isn't set:
+     *      - type: Integer type of directive, see HTMLPurifier_VarParser for definitions
+     *      - allow_null: If set, this directive allows null values
+     *      - aliases: If set, an associative array of value aliases to real values
+     *      - allowed: If set, a lookup array of allowed (string) values
+     *  - If isAlias is set:
+     *      - namespace: Namespace this directive aliases to
+     *      - name: Directive name this directive aliases to
+     * 
+     * This class is friendly with HTMLPurifier_Config. If you need introspection
+     * about the schema, you're better of using the ConfigSchema_Interchange,
+     * which uses more memory but has much richer information.
      */
     public $info = array();
 
@@ -21,15 +42,6 @@ class HTMLPurifier_ConfigSchema {
      */
     static protected $singleton;
 
-    /**
-     * Variable parser.
-     */
-    protected $parser;
-
-    public function __construct() {
-        $this->parser = new HTMLPurifier_VarParser_Flexible();
-    }
-
     /**
      * Unserializes the default ConfigSchema.
      */
@@ -62,11 +74,11 @@ public static function instance($prototype = null) {
      * @param $allow_null Whether or not to allow null values
      */
     public function add($namespace, $name, $default, $type, $allow_null) {
-        $default = $this->parser->parse($default, $type, $allow_null);
-        $this->info[$namespace][$name] = new HTMLPurifier_ConfigDef_Directive();
-        $this->info[$namespace][$name]->type = $type;
-        $this->info[$namespace][$name]->allow_null = $allow_null;
-        $this->defaults[$namespace][$name]   = $default;
+        $obj = new stdclass();
+        $obj->type = is_int($type) ? $type : HTMLPurifier_VarParser::$types[$type];
+        if ($allow_null) $obj->allow_null = true;
+        $this->info[$namespace][$name] = $obj;
+        $this->defaults[$namespace][$name] = $default;
     }
 
     /**
@@ -90,6 +102,9 @@ public function addNamespace($namespace) {
      * @param $aliases Hash of aliased values to the real alias
      */
     public function addValueAliases($namespace, $name, $aliases) {
+        if (!isset($this->info[$namespace][$name]->aliases)) {
+            $this->info[$namespace][$name]->aliases = array();
+        }
         foreach ($aliases as $alias => $real) {
             $this->info[$namespace][$name]->aliases[$alias] = $real;
         }
@@ -104,7 +119,6 @@ public function addValueAliases($namespace, $name, $aliases) {
      * @param $allowed Lookup array of allowed values
      */
     public function addAllowedValues($namespace, $name, $allowed) {
-        $type = $this->info[$namespace][$name]->type;
         $this->info[$namespace][$name]->allowed = $allowed;
     }
 
@@ -116,15 +130,18 @@ public function addAllowedValues($namespace, $name, $allowed) {
      * @param $new_name Directive that the alias will be to
      */
     public function addAlias($namespace, $name, $new_namespace, $new_name) {
-        $this->info[$namespace][$name] = new HTMLPurifier_ConfigDef_DirectiveAlias($new_namespace, $new_name);
+        $obj = new stdclass;
+        $obj->namespace = $new_namespace;
+        $obj->name = $new_name;
+        $obj->isAlias = true;
+        $this->info[$namespace][$name] = $obj;
     }
 
     // DEPRECATED METHODS
 
     /** @see HTMLPurifier_ConfigSchema->set() */
     public static function define($namespace, $name, $default, $type, $description) {
         HTMLPurifier_ConfigSchema::deprecated(__METHOD__);
-        // process modifiers (OPTIMIZE!)
         $type_values = explode('/', $type, 2);
         $type = $type_values[0];
         $modifier = isset($type_values[1]) ? $type_values[1] : false;
@@ -168,7 +185,8 @@ public static function defineAlias($namespace, $name, $new_namespace, $new_name)
     /** @deprecated, use HTMLPurifier_VarParser->parse() */
     public function validate($a, $b, $c = false) {
         trigger_error("HTMLPurifier_ConfigSchema->validate deprecated, use HTMLPurifier_VarParser->parse instead", E_USER_NOTICE);
-        return $this->parser->parse($a, $b, $c);
+        $parser = new HTMLPurifier_VarParser();
+        return $parser->parse($a, $b, $c);
     }
 
     /**

diff --git a/library/HTMLPurifier/ConfigSchema/Validator.php b/library/HTMLPurifier/ConfigSchema/Validator.php
@@ -111,7 +111,8 @@ public function validateDirective($d) {
         if (!is_null($d->allowed) || !empty($d->valueAliases)) {
             // allowed and valueAliases require that we be dealing with
             // strings, so check for that early.
-            if (!isset(HTMLPurifier_VarParser::$stringTypes[$d->type])) {
+            $d_int = HTMLPurifier_VarParser::$types[$d->type];
+            if (!isset(HTMLPurifier_VarParser::$stringTypes[$d_int])) {
                 $this->error('type', 'must be a string type when used with allowed or value aliases');
             }
         }

diff --git a/library/HTMLPurifier/ConfigSchema/schema.ser b/library/HTMLPurifier/ConfigSchema/schema.ser