From 6e81f63635fb8f60a3a51af811832db735c8e558 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 9 Jul 2015 17:21:45 +0200 Subject: [PATCH 1/6] Reduce memory footprint of rules by storing data in blob Not declaring the job property saves significant amounts of memory as most rules leave it as null --- src/Composer/DependencyResolver/Problem.php | 2 +- src/Composer/DependencyResolver/Rule.php | 61 +++++++++---------- src/Composer/DependencyResolver/RuleSet.php | 1 - src/Composer/DependencyResolver/Solver.php | 6 +- .../Test/DependencyResolver/RuleTest.php | 13 +--- 5 files changed, 37 insertions(+), 46 deletions(-) diff --git a/src/Composer/DependencyResolver/Problem.php b/src/Composer/DependencyResolver/Problem.php index a1910c3ed..7fcb636b0 100644 --- a/src/Composer/DependencyResolver/Problem.php +++ b/src/Composer/DependencyResolver/Problem.php @@ -47,7 +47,7 @@ class Problem */ public function addRule(Rule $rule) { - $this->addReason($rule->getId(), array( + $this->addReason(spl_object_hash($rule), array( 'rule' => $rule, 'job' => $rule->getJob(), )); diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index a19c121c2..c34015b52 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -35,27 +35,22 @@ class Rule */ public $literals; - protected $disabled; - protected $type; - protected $id; - protected $reason; + protected $blob; protected $reasonData; - protected $job; - public function __construct(array $literals, $reason, $reasonData, $job = null) { // sort all packages ascending by id sort($literals); $this->literals = $literals; - $this->reason = $reason; $this->reasonData = $reasonData; - $this->disabled = false; - $this->job = $job; - $this->type = -1; + if ($job) { + $this->job = $job; + } + $this->blob = pack('ccc', -1, $reason, false); } public function getHash() @@ -64,24 +59,14 @@ class Rule return $data['hash']; } - public function setId($id) - { - $this->id = $id; - } - - public function getId() - { - return $this->id; - } - public function getJob() { - return $this->job; + return isset($this->job) ? $this->job : null; } public function getReason() { - return $this->reason; + return $this->getBlob('a2'); } public function getReasonData() @@ -91,11 +76,11 @@ class Rule public function getRequiredPackage() { - if ($this->reason === self::RULE_JOB_INSTALL) { + if ($this->getReason() === self::RULE_JOB_INSTALL) { return $this->reasonData; } - if ($this->reason === self::RULE_PACKAGE_REQUIRES) { + if ($this->getReason() === self::RULE_PACKAGE_REQUIRES) { return $this->reasonData->getTarget(); } } @@ -125,32 +110,32 @@ class Rule public function setType($type) { - $this->type = $type; + return $this->setBlob('a1', $type); } public function getType() { - return $this->type; + return $this->getBlob('a1'); } public function disable() { - $this->disabled = true; + return $this->setBlob('a3', true); } public function enable() { - $this->disabled = false; + return $this->setBlob('a3', false); } public function isDisabled() { - return $this->disabled; + return (bool) $this->getBlob('a3'); } public function isEnabled() { - return !$this->disabled; + return !$this->getBlob('a3'); } /** @@ -176,7 +161,7 @@ class Rule $ruleText .= $pool->literalToPrettyString($literal, $installedMap); } - switch ($this->reason) { + switch ($this->getReason()) { case self::RULE_INTERNAL_ALLOW_UPDATE: return $ruleText; @@ -268,6 +253,20 @@ class Rule return implode(', ', $prepared); } + private function getBlob($var) + { + $current = unpack('c3a', $this->blob); + return $current[$var]; + } + + private function setBlob($var, $value) + { + $current = unpack('c3a', $this->blob); + $current[$var] = $value; + array_unshift($current, 'ccc'); + $this->blob = call_user_func_array('pack', $current); + } + /** * Formats a rule as a string of the format (Literal1|Literal2|...) * diff --git a/src/Composer/DependencyResolver/RuleSet.php b/src/Composer/DependencyResolver/RuleSet.php index b9545123f..02da881e8 100644 --- a/src/Composer/DependencyResolver/RuleSet.php +++ b/src/Composer/DependencyResolver/RuleSet.php @@ -66,7 +66,6 @@ class RuleSet implements \IteratorAggregate, \Countable $this->ruleById[$this->nextRuleId] = $rule; $rule->setType($type); - $rule->setId($this->nextRuleId); $this->nextRuleId++; $hash = $rule->getHash(); diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index d0f7ec8fb..f8f316269 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -319,7 +319,7 @@ class Solver $this->rules->add($newRule, RuleSet::TYPE_LEARNED); - $this->learnedWhy[$newRule->getId()] = $why; + $this->learnedWhy[spl_object_hash($newRule)] = $why; $ruleNode = new RuleWatchNode($newRule); $ruleNode->watch2OnHighest($this->decisions); @@ -454,7 +454,7 @@ class Solver private function analyzeUnsolvableRule($problem, $conflictRule) { - $why = $conflictRule->getId(); + $why = spl_object_hash($conflictRule); if ($conflictRule->getType() == RuleSet::TYPE_LEARNED) { $learnedWhy = $this->learnedWhy[$why]; @@ -572,7 +572,7 @@ class Solver private function enableDisableLearnedRules() { foreach ($this->rules->getIteratorFor(RuleSet::TYPE_LEARNED) as $rule) { - $why = $this->learnedWhy[$rule->getId()]; + $why = $this->learnedWhy[spl_object_hash($rule)]; $problemRules = $this->learnedPool[$why]; $foundDisabled = false; diff --git a/tests/Composer/Test/DependencyResolver/RuleTest.php b/tests/Composer/Test/DependencyResolver/RuleTest.php index 4382987eb..f71af15bd 100644 --- a/tests/Composer/Test/DependencyResolver/RuleTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleTest.php @@ -13,6 +13,7 @@ namespace Composer\Test\DependencyResolver; use Composer\DependencyResolver\Rule; +use Composer\DependencyResolver\RuleSet; use Composer\DependencyResolver\Pool; use Composer\Repository\ArrayRepository; use Composer\TestCase; @@ -34,14 +35,6 @@ class RuleTest extends TestCase $this->assertEquals($hash['hash'], $rule->getHash()); } - public function testSetAndGetId() - { - $rule = new Rule(array(), 'job1', null); - $rule->setId(666); - - $this->assertEquals(666, $rule->getId()); - } - public function testEqualsForRulesWithDifferentHashes() { $rule = new Rule(array(1, 2), 'job1', null); @@ -69,9 +62,9 @@ class RuleTest extends TestCase public function testSetAndGetType() { $rule = new Rule(array(), 'job1', null); - $rule->setType('someType'); + $rule->setType(RuleSet::TYPE_JOB); - $this->assertEquals('someType', $rule->getType()); + $this->assertEquals(RuleSet::TYPE_JOB, $rule->getType()); } public function testEnable() From 961ea868ac63c38f22ed9d912c98bed06b005935 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 9 Jul 2015 18:26:31 +0200 Subject: [PATCH 2/6] Use an integer bitfield for rule properties instead of a string binary blob --- src/Composer/DependencyResolver/Rule.php | 26 ++++++++++----------- src/Composer/DependencyResolver/RuleSet.php | 4 ++-- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index c34015b52..a45205246 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -50,7 +50,7 @@ class Rule $this->job = $job; } - $this->blob = pack('ccc', -1, $reason, false); + $this->blob = (0 << 16) | ($reason << 8) | (255 << 0); } public function getHash() @@ -66,7 +66,7 @@ class Rule public function getReason() { - return $this->getBlob('a2'); + return $this->getBlob(1); } public function getReasonData() @@ -110,32 +110,32 @@ class Rule public function setType($type) { - return $this->setBlob('a1', $type); + return $this->setBlob(0, $type); } public function getType() { - return $this->getBlob('a1'); + return $this->getBlob(0); } public function disable() { - return $this->setBlob('a3', true); + return $this->setBlob(2, 1); } public function enable() { - return $this->setBlob('a3', false); + return $this->setBlob(2, 0); } public function isDisabled() { - return (bool) $this->getBlob('a3'); + return (bool) $this->getBlob(2); } public function isEnabled() { - return !$this->getBlob('a3'); + return !$this->getBlob(2); } /** @@ -255,16 +255,14 @@ class Rule private function getBlob($var) { - $current = unpack('c3a', $this->blob); - return $current[$var]; + $offset = 8*$var; + return ($this->blob & (255 << $offset)) >> $offset; } private function setBlob($var, $value) { - $current = unpack('c3a', $this->blob); - $current[$var] = $value; - array_unshift($current, 'ccc'); - $this->blob = call_user_func_array('pack', $current); + $offset = 8*$var; + $this->blob = ($this->blob & ~(255 << $offset)) | ((255 & $value) << $offset); } /** diff --git a/src/Composer/DependencyResolver/RuleSet.php b/src/Composer/DependencyResolver/RuleSet.php index 02da881e8..26771cef6 100644 --- a/src/Composer/DependencyResolver/RuleSet.php +++ b/src/Composer/DependencyResolver/RuleSet.php @@ -30,7 +30,7 @@ class RuleSet implements \IteratorAggregate, \Countable public $ruleById; protected static $types = array( - -1 => 'UNKNOWN', + 255 => 'UNKNOWN', self::TYPE_PACKAGE => 'PACKAGE', self::TYPE_JOB => 'JOB', self::TYPE_LEARNED => 'LEARNED', @@ -130,7 +130,7 @@ class RuleSet implements \IteratorAggregate, \Countable public function getTypes() { $types = self::$types; - unset($types[-1]); + unset($types[255]); return array_keys($types); } From 329ab5cf41fd6c2fe5fd037d59e5153dd009d8e3 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 9 Jul 2015 18:44:56 +0200 Subject: [PATCH 3/6] Rename blob rule property to bitfield --- src/Composer/DependencyResolver/Rule.php | 26 ++++++++++++------------ 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index a45205246..e0f6ac138 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -35,7 +35,7 @@ class Rule */ public $literals; - protected $blob; + protected $bitfield; protected $reasonData; public function __construct(array $literals, $reason, $reasonData, $job = null) @@ -50,7 +50,7 @@ class Rule $this->job = $job; } - $this->blob = (0 << 16) | ($reason << 8) | (255 << 0); + $this->bitfield = (0 << 16) | ($reason << 8) | (255 << 0); } public function getHash() @@ -66,7 +66,7 @@ class Rule public function getReason() { - return $this->getBlob(1); + return $this->getBitfield(1); } public function getReasonData() @@ -110,32 +110,32 @@ class Rule public function setType($type) { - return $this->setBlob(0, $type); + return $this->setBitfield(0, $type); } public function getType() { - return $this->getBlob(0); + return $this->getBitfield(0); } public function disable() { - return $this->setBlob(2, 1); + return $this->setBitfield(2, 1); } public function enable() { - return $this->setBlob(2, 0); + return $this->setBitfield(2, 0); } public function isDisabled() { - return (bool) $this->getBlob(2); + return (bool) $this->getBitfield(2); } public function isEnabled() { - return !$this->getBlob(2); + return !$this->getBitfield(2); } /** @@ -253,16 +253,16 @@ class Rule return implode(', ', $prepared); } - private function getBlob($var) + private function getBitfield($var) { $offset = 8*$var; - return ($this->blob & (255 << $offset)) >> $offset; + return ($this->bitfield & (255 << $offset)) >> $offset; } - private function setBlob($var, $value) + private function setBitfield($var, $value) { $offset = 8*$var; - $this->blob = ($this->blob & ~(255 << $offset)) | ((255 & $value) << $offset); + $this->bitfield = ($this->bitfield & ~(255 << $offset)) | ((255 & $value) << $offset); } /** From f535542fcac3365ca8f237874070bd54e9085d52 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 9 Jul 2015 18:59:16 +0200 Subject: [PATCH 4/6] Use constants with names for bitfield offsets --- src/Composer/DependencyResolver/Rule.php | 28 ++++++++++++++---------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index e0f6ac138..776cd1455 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -29,6 +29,10 @@ class Rule const RULE_LEARNED = 12; const RULE_PACKAGE_ALIAS = 13; + const BITFIELD_TYPE = 0; + const BITFIELD_REASON = 8; + const BITFIELD_DISABLED = 16; + /** * READ-ONLY: The literals this rule consists of. * @var array @@ -50,7 +54,9 @@ class Rule $this->job = $job; } - $this->bitfield = (0 << 16) | ($reason << 8) | (255 << 0); + $this->bitfield = (0 << self::BITFIELD_DISABLED) | + ($reason << self::BITFIELD_REASON) | + (255 << self::BITFIELD_TYPE); } public function getHash() @@ -66,7 +72,7 @@ class Rule public function getReason() { - return $this->getBitfield(1); + return $this->getBitfield(self::BITFIELD_REASON); } public function getReasonData() @@ -110,32 +116,32 @@ class Rule public function setType($type) { - return $this->setBitfield(0, $type); + return $this->setBitfield(self::BITFIELD_TYPE, $type); } public function getType() { - return $this->getBitfield(0); + return $this->getBitfield(self::BITFIELD_TYPE); } public function disable() { - return $this->setBitfield(2, 1); + return $this->setBitfield(self::BITFIELD_DISABLED, 1); } public function enable() { - return $this->setBitfield(2, 0); + return $this->setBitfield(self::BITFIELD_DISABLED, 0); } public function isDisabled() { - return (bool) $this->getBitfield(2); + return (bool) $this->getBitfield(self::BITFIELD_DISABLED); } public function isEnabled() { - return !$this->getBitfield(2); + return !$this->getBitfield(self::BITFIELD_DISABLED); } /** @@ -253,15 +259,13 @@ class Rule return implode(', ', $prepared); } - private function getBitfield($var) + private function getBitfield($offset) { - $offset = 8*$var; return ($this->bitfield & (255 << $offset)) >> $offset; } - private function setBitfield($var, $value) + private function setBitfield($offset, $value) { - $offset = 8*$var; $this->bitfield = ($this->bitfield & ~(255 << $offset)) | ((255 & $value) << $offset); } From 956035e64113c985ee7254e6f8f95f1dffdb240f Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Thu, 9 Jul 2015 19:40:03 +0200 Subject: [PATCH 5/6] Remove the unnecessary return statements from setters --- src/Composer/DependencyResolver/Rule.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index 776cd1455..f129c6f66 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -116,7 +116,7 @@ class Rule public function setType($type) { - return $this->setBitfield(self::BITFIELD_TYPE, $type); + $this->setBitfield(self::BITFIELD_TYPE, $type); } public function getType() @@ -126,12 +126,12 @@ class Rule public function disable() { - return $this->setBitfield(self::BITFIELD_DISABLED, 1); + $this->setBitfield(self::BITFIELD_DISABLED, 1); } public function enable() { - return $this->setBitfield(self::BITFIELD_DISABLED, 0); + $this->setBitfield(self::BITFIELD_DISABLED, 0); } public function isDisabled() From cf1af585148e4f2c2a7321d59da7cb988b1ba117 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 14 Jul 2015 14:18:50 +0200 Subject: [PATCH 6/6] Use bitwise operators directly in rules instead of get/set Bitfield --- src/Composer/DependencyResolver/Rule.php | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index f129c6f66..5d95d678a 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -72,7 +72,7 @@ class Rule public function getReason() { - return $this->getBitfield(self::BITFIELD_REASON); + return ($this->bitfield & (255 << self::BITFIELD_REASON)) >> self::BITFIELD_REASON; } public function getReasonData() @@ -116,32 +116,32 @@ class Rule public function setType($type) { - $this->setBitfield(self::BITFIELD_TYPE, $type); + $this->bitfield = ($this->bitfield & ~(255 << self::BITFIELD_TYPE)) | ((255 & $type) << self::BITFIELD_TYPE); } public function getType() { - return $this->getBitfield(self::BITFIELD_TYPE); + return ($this->bitfield & (255 << self::BITFIELD_TYPE)) >> self::BITFIELD_TYPE; } public function disable() { - $this->setBitfield(self::BITFIELD_DISABLED, 1); + $this->bitfield = ($this->bitfield & ~(255 << self::BITFIELD_DISABLED)) | (1 << self::BITFIELD_DISABLED); } public function enable() { - $this->setBitfield(self::BITFIELD_DISABLED, 0); + $this->bitfield = $this->bitfield & ~(255 << self::BITFIELD_DISABLED); } public function isDisabled() { - return (bool) $this->getBitfield(self::BITFIELD_DISABLED); + return (bool) (($this->bitfield & (255 << self::BITFIELD_DISABLED)) >> self::BITFIELD_DISABLED); } public function isEnabled() { - return !$this->getBitfield(self::BITFIELD_DISABLED); + return !(($this->bitfield & (255 << self::BITFIELD_DISABLED)) >> self::BITFIELD_DISABLED); } /** @@ -259,16 +259,6 @@ class Rule return implode(', ', $prepared); } - private function getBitfield($offset) - { - return ($this->bitfield & (255 << $offset)) >> $offset; - } - - private function setBitfield($offset, $value) - { - $this->bitfield = ($this->bitfield & ~(255 << $offset)) | ((255 & $value) << $offset); - } - /** * Formats a rule as a string of the format (Literal1|Literal2|...) *