[X] Помощник

[Youshi]

Какое-то время назад мне в руки попал исходник сей прекрасной системы. Ни в коем разе не хочу умалять ее достоинства, но все же есть одно очень серьезное замечание.

Откуда у меня исходник, и не спрашивайте - сервер взломал и скачал. Не в том суть. А суть вот в чем.

В системе прямо в индексе есть вот такой кусочек кода:

define('CMS',true);
require 'classes/mainclass.php';
$Mainclass=new MainClass();
$Mainclass->start_time=microtime();#Начало отсчета времени движка.

Вроде бы ничего противозаконного тут нет, НО!
...взгляните на четвертую строчку.

для тех, кто не знает пхп, поясняю - в ней запоминается время старта скрипта.
так вот. это время не учитывает:

а. включение файла classes/mainclass.php (одна файловая операция)
б. создание объекта $Mainclass - загрузка класса в память и выполнение конструктора класса
в. в конструкторе есть вот такой кусочек кода:

require $root_path.'config_general.php';
//пропущено
//....
//самая ресурсоемкая процедура всей системы
foreach($includes as $tfile)

		{			require_once $root_path.'classes/class_'.$tfile.'.php';

			//пропущено
//....

$this->Db->Query('SELECT * FROM `f_services`',__file__,__line__);
			while ($arr=$this->Db->Fetch_assoc())
				$this->services[$arr['id']]=$arr;

то есть: включение еще одного файлика (уже 2 файловых операции)
потом подключение файлов в цикле (еще плюс n файловых операций, где n-число итераций цикла)
ну и в конце один запросик в БД и снова итерация, итерация, итерация....

Это я все к чему. Это к тому, что во времени генерации системы не учитывается время на ее инициализацию. а в большинстве случаев именно на это уходит большая часть работы. Отсюда вывод - время генерации ЦМС (за которым так гонится многоуважаемый Sunvas) сильно занижено!

Выводы делаем сами
UPD: так лучше?

ЗЫ
Это не пиар и не попытка отбить у кого-то охоту пользоваться системой. Это скорее повод для размышлений ее автора...

ЗЫЗЫ
Ну не может быть во всем все замечательно. Даже компьютеры ошибаются... ибо созданы людьми, которым ошибки свойственны... ­

ЗЫЗЫЗЫ
Саня! Еще раз тебя с юбилеем!

Сообщение отредактировал Youshi - 2008-12-14, 22:09

[Гость_RULEVOY_*]

заюзал ../../../../../etc/ ?

И какие мне лично выводы сделать? Делаю вывод, что просто щас закроют дыру, поставят считалку в "до" .
Время загрузки изменится на несколько милисикунд, собственно и все.
А такие темы создавать нужно в нормальном тоне, а не писать что мол делайте выводы а в конце с праздником поздравлять .

Сообщение отредактировал RULEVOY - 2008-12-14, 21:54

[Youshi]

Тон нормальный.
Я никого оскорблять не собирался.

заюзал ../../../../../etc/ ?

Не важно...

Время загрузки изменится на несколько милисикунд, собственно и все.

Не шаришь - не суди. Я кучу времени убил на то, чтобы уменьшить эффект от инклудов. И уж поверь - они офигенно замедляют.

поставят считалку в "до"

Собственно это и было целью.

Хотя мож и правда грубовато написал. Эдакое прям разоблачение на пустом месте получилось
Если что - извиняюсь... я больше так не буду...

PS
сначала хотел по-тихому в асе с Sunvas`ом переговорить. А потом передумал... Хочу, чтобы меня тут камнями закидали

Сообщение отредактировал Youshi - 2008-12-14, 22:16

[Youshi]

ЗЫ
перенесите в найденные ошибки

[Alexander]

Исправил.
Действительно, конкретно ступил.. Время, которое я "украл" составило - 0.012 сек.. Что ж... Надо будет искать варианты поиска оптимальных решений. Слить все в один файл - конечно вариант, но слишком уж неудобный код получится.

Вообще не факт, что время, затрачиваемое на генерацию страницы служит реальным показателем нагрузки на сервер. - Ведь в данном случае речь идет о поиске и считывании файла винтом; процессор в это время отдыхает. Или я не прав?

Сообщение отредактировал Alexander - 2008-12-14, 22:46

[Youshi]

Слить все в один файл - конечно вариант, но слишком уж неудобный код получится.

Ни в коем случае этого не делай! Один класс - один файл.
К тому же я проводил такие эксперименты - ничего хорошего это не дало. А кодить стало ужасно неудобно.

Вообще не факт, что время, затрачиваемое на генерацию страницы служит реальным показателем нагрузки на сервер

Да в принципе вообще не служит. Показатели тут другие - нагрузка на бд, использование памяти и процессорного времени, количество файловых операций (а обычно именно это и оказывается тем самым "бутылочным горлышком").

процессор в это время отдыхает

процессор управляет этим самым поиском. на то он и процессор. и тут многое от файловой системы зависит, от самого винта (винтов если раид)

[Гость_RULEVOY_*]

Не шаришь - не суди. Я кучу времени убил на то, чтобы уменьшить эффект от инклудов. И уж поверь - они офигенно замедляют.

да шарю вобще-то Но время выполнения итерации зависит не только от исполняемого кода, а еще и от сервера и от много чего. eAccelerator используем? Вобщем посмотрим сколько времени займет всеэто дело после исправления, перед оптимизацией.

/*Добавлено позже*/
а вот и ответ собственно...

...Время, которое я "украл" составило - 0.012 сек..

сначала хотел по-тихому в асе с Sunvas`ом переговорить. А потом передумал... Хочу, чтобы меня тут камнями закидали

потихонькому - это по-человечески вроде бы.

Сообщение отредактировал RULEVOY - 2008-12-14, 22:53

[Youshi]

потихонькому - это по-человечески вроде бы.

Согласен.
А с чего ты взял, что я человек?
Ну лан. что сделано - то сделано.

да шарю вобще-то Но время выполнения итерации зависит не только от исполняемого кода, а еще и от сервера и от много чего. eAccelerator используем? Вобщем посмотрим сколько времени займет всеэто дело после исправления, перед оптимизацией.

ну я на итерации упора и не делал (хотя одна итерация в цикле съедает гораздо больше команд процессора, нежели чем просто повтор одного и того же участка кода. только нет смысла считать эти самые команды. в рамках всей системы не критично и сильно зависит от архитектуры процессора). а инклуд - это да. чем больше - тем хуже, ибо грузит уже файловую подсистему
eAccelerator - хорошая штука и жизнь несомненно облегчит. но ведь не везде он стоит.

[Alexander]

Все это, конечно, хорошо... Но давайте ближе к телу. Вот весь код конструктора системы. ПодскАжите, как его оптимизировать?

	/*
		Кнструктор класса
		$init_other_classes (bool) - автоматическая загрузка остальные классы
		$classes (array) - если $init_other_classes=false, то в этом массиве содержаться имена
						классов (без префикса "class_"), которые следует загрузить, если же
						$init_other_classes=true, то тут содежаться имена классов, которые
						НЕ будут загружены
		$root_path - путь для инклуда. Не важно есть ли слэш.
	*/
	function __construct($init_other_classes=true, $classes=array(), $root_path=false)
	{
		unset($this->Mainclass);
		if($root_path===false)
			$root_path=getcwd();
		$root_path=$this->CheckSlash($root_path);#Проверка, есть ли слеш в конце, если нет - добавим.
		$this->root_path=$root_path;
		$this->IPDetect();#Определим IP адрес
		define('SC',true);
		require $root_path.'config_general.php'; #Подгрузим конфиг
		foreach($general as $akey=>$value)
			if(!isset($this->$akey))
			{
				$akey=strtolower($akey);
				$this->$akey=$value;
			}
		$includes=glob($root_path.'classes/class_*.php');
		foreach($includes as $v)
		{
			if(preg_match('#class_([a-z0-9]+)\.php$#',$v,$cl)==0)
				continue;
			$cl=$cl[1];
			if ($init_other_classes xor !in_array($cl,$classes))
				continue;
			require_once $v;
			$cl=ucfirst($cl);
			$this->$cl=new $cl(&$this);
		}
		$this->LoadLanguage('langs','main','main-%');#Загрузка языкового файла
		$this->domain=$this->MyGetEnv('HTTP_HOST');
		if(is_object($this->Db))
		{
			$this->Db->Connect();
			$this->Db->Query('SELECT * FROM `f_services`',__file__,__line__);
			while ($arr=$this->Db->Fetch_assoc())
				$this->services[$arr['id']]=$arr;
		}
		$this->os=strncasecmp(PHP_OS,'win',3)==0 ? 'w' : 'u';
	}

$x='dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddq kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk iiiiiiiiiiiiiiiiiiiiiii uuuuuuuuuuuuuuuuuuuuuuuuuuuu ddddddddddddd ggggggggggggggggggggggggggggggggggggggggggg hhhhhhhhhhhhhhhhhhhhhhhhhhhhh';

Тааак. Вот обнаружился еще один глюк форума.. Отпишу Сергею..

Сообщение отредактировал Alexander - 2008-12-15, 17:26

[Гость_RULEVOY_*]

Согласен.
А с чего ты взял, что я человек?
Ну лан. что сделано - то сделано.

ты слишком самокритичен

ну я на итерации упора и не делал (хотя одна итерация в цикле съедает гораздо больше команд процессора, нежели чем просто повтор одного и того же участка кода. только нет смысла считать эти самые команды. в рамках всей системы не критично и сильно зависит от архитектуры процессора). а инклуд - это да. чем больше - тем хуже, ибо грузит уже файловую подсистему
eAccelerator - хорошая штука и жизнь несомненно облегчит. но ведь не везде он стоит.

Предложи свои варианты, меня заинтересовала тема, собственно вот еще по сабжу:
http://ua.php.net/autoload

[Alexander]

Предложи свои варианты, меня заинтересовала тема, собственно вот еще по сабжу:
http://ua.php.net/autoload

Хм.. Едва-ли это поможет делу. Те же инклуды, только в другом флаконе.

[Youshi]

Хм.. Едва-ли это поможет делу. Те же инклуды, только в другом флаконе.

Флаконы бывают разные... черные, белые, красные...

По сабжу.

ПодскАжите, как его оптимизировать?

while($tfile=readdir($hdir))
			if (is_file($root_path.'classes/'.$tfile) and
				preg_match('%class_([a-z0-9]+)\.php%',$tfile,$match) and
				($init_other_classes xor array_search($match[1],$classes)!==false))
					$includes[]=$match[1];
		closedir($hdir);
		sort($includes);
		foreach($includes as $tfile)
		{			require_once $root_path.'classes/class_'.$tfile.'.php';
			$tfile=ucfirst($tfile);
			$this->$tfile=new $tfile(&$this);		}

зачем такой мазохизм? можно ведь все это в одном цикле проделать. плюс у тебя есть функция __autoload(), которая с этой задачей тоже очень неплохо справляется. только есть один хитрый способ, как ее оптимизировать донельзя. тогда вышеприведенный кусок кода тебе вообще не понадобится. требование тут только одно - явный вызов конструкторов классов и полный упор на __autoload()

а собственно способ вот такой:

define ('DIR_1', 'dir_1');
define ('DIR_2', 'dir_2');
define ('DIR_3', 'dir_1/dir_3');
//и так далее
function __autoload ($class_name)
{
	static $paths = array();
	//если массив путей не заполнен - заполняем
  if (empty($paths))
  {
	  $tmp = glob(
		'{'.implode(',', array(DIR_1, DIR_2, DIR_3)).'}/class_*.php',
	  GLOB_BRACE
	);
	foreach ($tmp as $file_name)
	{
		$paths[substr(strrchr($file_name,'/class_'), 7, -4)] = $file_name;
	}
  }
	$class = strtolower($class_name);
	
	if (isset($paths[$class]))
	{
		require($paths[$class]);
  }
  else
  {
	  trigger_error('Невозможно загрузить класс "'.$class_name.'"', E_USER_WARNING);
  }
}

я его использую в своей системе... это пока первая мысль. появятся еще - отпишу

Хм.. Едва-ли это поможет делу. Те же инклуды, только в другом флаконе.

Да что вы все на инклудах зациклились? Они были, есть, и будут есть...

Сообщение отредактировал Youshi - 2008-12-14, 23:29

[Alexander]

зачем такой мазохизм?

Вобщем-то того участока кода уже нет. Сейчас все выглядит так:

		$includes=glob($root_path.'classes/class_*.php');
		foreach($includes as $v)
		{
			if(preg_match('#class_([a-z0-9]+)\.php$#',$v,$cl)==0)
				continue;
			$cl=$cl[1];
			if ($init_other_classes xor !in_array($cl,$classes))
				continue;
			require_once $v;
			$cl=ucfirst($cl);
			$this->$cl=new $cl(&$this);
		}

Это тоже мазохизм?

Спасибо за функцию автолоада - почерпну идею

[Youshi]

Вобщем-то того участока кода уже нет. Сейчас все выглядит так:

Ну у меня старая версия )))))

if(preg_match('#class_([a-z0-9]+)\.php$#',$v,$cl)==0)

- это, я думаю, ни к чему. PCRE - довольно затратная штука. а если кто-то умудрится залить тебе в папку с классами какую-нить гадость, ..... сам понимаешь
наверное стоило бы заменить ее на обычные строковые функции - только для того, чтобы выделить имя класса. к тому же часть работы за нее уже сделаа glob


require_once работает медленнее, чем обычный require. а в данном контексте ты ну никак не сможешь подключить один и тот же класс дважды. если полностью перейдешь на использование __autoload () для загрузки всех остальных классов, то вообще беспокоиться ни о чем не надо будет, т. к. она вызывается только, если класс еще не подключен.

Сообщение отредактировал Youshi - 2008-12-14, 23:50

[Next]

if(preg_match('#class_([a-z0-9]+)\.php$#',$v,$cl)==0)

Может наоборот если совпадение будет?

[Youshi]

Может наоборот если совпадение будет?

Честно говоря, я не очень понял, о чем ты. Но если понял, то нет. Все верно. Если совпадения нет, пропускаем файл и проверяем следующий.

[Alexander]

После манипуляций получилось:

		$arr=array(
					'blocks',
					'cache',
					'db',
					'functions',
					'security',
					'strings',
					'template',
					'url',
		);
		foreach($arr as $v)
			if ($init_other_classes xor in_array($v,$classes))
			{
				$v=ucfirst($v);
				$this->$v=new $v(&$this);
			}

[Youshi]

После манипуляций получилось:

А как насчет вот такой конструкции:
если конечно я правильно понял логику

if (!$init_other_classes) $arr = array_diff ($classes, $arr);
		foreach($arr as $v)
				{
				$v=ucfirst($v);
				$this->$v=new $v(&$this);
				}

[Next]

Если совпадения нет, пропускаем файл и проверяем следующий.

тогда я не понял ее смысл...

А как насчет вот такой конструкции

если у него условия else нету, то так намного проще

[Alexander]

Youshi, смысл в чем: если $init_other_classes===true, мы грузим все классы, кроме тех, которые указаны в $classes; если же $init_other_classes===false - мы грузим только классы, находящиеся внутри $classes