Рефакторинг и оптимизация растрового рендера

Рефакторинг – это улучшение читаемости кода без изменения его функционала.
Оптимизация – увеличение эффективности работы программы.

В этой публикации я хочу на небольшом примере показать, как происходит рефакторинг и оптимизация. Никакой теории не будет, так как она уже давно описана в большом количестве книг. Перейдем сразу к практике. Для примера возьмём симпатичную флешку из публикации Растровый рендер в as3. Двигаем тысячи картинок с разрешения Platon. Если посмотреть на код, то можно увидеть, что внутри не всё так симпатично. Это нам и нужно. Предлагаю не закрывать статью по ссылке выше, чтобы исходный код был у вас перед глазами. Будем улучшать.

xorRand.as

Рефакторинг
Первое, что бросается в глаза – это название класса. Оно с маленькой буквы.
В ActionScript давно сложилось так, что названия даются в соответствии с CamelCase. Вообще, тут надо немного отвлечься и сказать, что существуют различные стили кодирования. Набор правил, определяющих стиль кодирования, описывается в документе под названием Coding conventions. Для ActionScript 3.0 существует Flex SDK coding conventions от Adobe и будем придерживаться его, а в соответствии с ним, требуется, чтобы названия классов писались с большой буквы. И вообще, типы данных с большой буквы просто принято писать. Меняем название на XORRandom.
Flex SDK coding conventions and best practices
Перевод

Рефакторинг
Поменяем название функции XORRandom на random. Согласитесь, XORRandom.random выглядит приятнее, чем XORRandom.XORRandom. Названия функций пишутся с маленькой буквы, это раз. Незачем второй раз указывать, что рандом получен именно при помощи операции XOR, из названия класса и так понятно, это два.

Оптимизация
Заметим, что в функции randRage() используется Math.floor(). В Параметрах всегда передается целое число, не выходящее за пределы int, следовательно, можно без проблем заменить Math.floor() на int().

return int(random * (max - min + 1)) + min;


Рефакторинг
Меняем название функции с randRange() на randomRangeInt(), чтобы чётко было ясно, что функция возвращает int. Хотя, это можно понять, взглянув на сигнатуру функции, но, когда читаешь код, не заглядываешь в описание каждой функции. И по началу я подумал, что функция возвращает любое число в заданном интервале, а не только целое.

Рефакторинг
Ещё уберем лишние пустые строчки и комментарии, добавим пустых разделительных строк между методами, и допишем функцию получению случайного числа randomRangeNumber(). Она потом понадобится.

package utils
{
    public class XORRandom
    {
        private static const MAX_RATIO:Number = 1 / uint.MAX_VALUE;
        private static var r:uint = Math.random() * uint.MAX_VALUE;
        
        public static function randomRangeInt(min:int, max:int):int 
        {
            return int(random * (max - min + 1)) + min;
        }
        
        public static function randomRangeNumber(min:Number, max:Number):Number 
        {
            return random * (max - min) + min;
        }
        
        public static function get random():Number
        {
            r ^= (r << 21);
            r ^= (r >>> 35);
            r ^= (r << 4);
            return r * MAX_RATIO;
        }
    }
}


cube.as

Рефакторинг
Название класса обманывает, так как работать мы будет с кругами, и ещё с маленькой буквы. Переименовываем в Circle.

Рефакторинг
Убираем лишний импорт.
import com.greensock.easing.Linear;


Рефакторинг и оптимизация
Убираем геттеры и сеттеры.
public function get x():Number 
{
 return _x;
}
 
public function set x(value:Number):void 
{
 _x = value;
}

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

Рефакторинг
Вот это реально смешная строчка.
_this = this;

Удаляем строчку и переменную _this.

Оптимизация
Замечаем дублирование данных. В point.x и _x хранятся одни и те же данные. Тут конечно надо задуматься, не используется отдельная переменная для того, чтобы ускорить работу, так как обращение к полю объекта дольше, чем к переменной класса. Но изучив код, обнаруживаем, что за кадр обращение к переменной _x происходит один раз.

При чем, после этого выполняется вот такая строчка:
point = new Point(x, y);

Каждый раз при вызове такой строчки создается новый объект Point. Вспомним, что объектов у нас может быть больше тысячи, а вызывается это каждый кадр. Получается хорошая такая работа для Garbage Collector.

Можно было бы сделать так:
point.x = _x;
point.y = _y;

Но раз уж определились в нецелесообразности переменной _x, то будет лучше сразу все вычисления заносить в point.x. Аналогично поступаем с переменной _y.
С переменными destPoint, destX и destY та же история. Поступаем так же.

Рефакторинг
Меняем название destPoint на destination. Я догадываюсь по сокращению, что dest – это destination, но это может также быть destruction или что-либо ещё. Во-вторых незачем указывать, что это точка. Ну в общем-то и так понятно. И ещё я сторонник написания слов без сокращений. Ещё помню в школе на уроке биологии я почти все слова сокращал до пары букв, потому что учитель быстро читал и я не успевал записывать. А потом мне приходилось брать тетрадь у одноклассников, потому что я сам не мог расшифровать, что я записал. Это заставляет меня писать названия переменных полностью. Хотя, конечно, 60 символов в названии тоже не очень хорошо.

Рефакторинг
В классе Main есть такие строчки.
var cub:cube = new cube();
cub.init();

При этом init() больше нигде не вызывается. Помещаем функцию init() в конструктор cube(), который уже переименован в Circle().

Рефакторинг
var sizeXY:int = xorRand.randRange(1, 20);

1 и 20. Такие числа в коде называются магическими, потому что, пока в коде не разберешься, не поймешь, что они значат. Заносим в константы, даем название.
private static const MIN_RADIUS:int = 1;
private static const MAX_RIADIUS:int = 10;
var radius:int = XORRandom.randomRangeInt(MIN_RADIUS, MAX_RIADIUS);


Рефакторинг
Также выносим в константы максимальное и минимальное значение скорости.
private static const MIN_SPEED:int = 1;
private static const MAX_SPEED:int = 5;
var speed:Number = XORRandom.randomRangeNumber(MIN_SPEED, MAX_SPEED);

Тут используется randomRangeNumber() вместо randomRangeInt(), чтобы круги двигались разнообразнее.

Рефакторинг
Также просто предлагаю сравнить два куска кода.

//создаем шарик рандомного цвета, и сохраняем картинку
var sizeXY:int = xorRand.randRange(1, 20);
var shape:Shape = doDrawCircle(sizeXY);
bitmapData = new BitmapData(sizeXY, sizeXY, true, 0);
bitmapData.draw(shape);
rect = new Rectangle(0, 0, sizeXY, sizeXY);

private function doDrawCircle(size:uint):Shape {
    var child:Shape = new Shape();
    var halfSize:uint = size / 2;
    child.graphics.beginFill(generateRndColor);
    child.graphics.drawCircle(halfSize, halfSize, halfSize);
    child.graphics.endFill();
    return child;
}


var radius:int = XORRandom.randomRangeInt(MIN_RADIUS, MAX_RIADIUS);
var size:int = radius * 2;
var shape:Shape = getCircle(radius);
bitmapData = new BitmapData(size, size, true, 0);
bitmapData.draw(shape);
rectangle = new Rectangle(0, 0, size, size);

private function getCircle(radius:int):Shape
{
    var circle:Shape = new Shape();
    var graphics:Graphics = circle.graphics;
    graphics.beginFill(randomColor);
    graphics.drawCircle(radius, radius, radius);
    graphics.endFill();
    return circle;
}


Тут ещё есть небольшой выигрыш в скорости. Хотя он совсем мизерный, но можно упомянуть. Во-первых, умножение на 2 быстрее, чем деление. Во-вторых, сохранение graphics в переменную немного ускоряет работу.

Рефакторинг
Замечаем, что код расчёта начальных координат очень похож на код в функции setNewDirection(). Объединяем в одну функцию. Немного позже покажу её. На самом деле я немного схитрил, так как код не совсем похож. Просто теперь для расчёта начального положения круга и расчета новых координат будет использваться один метод.

Рефакторинг
case 1://право
case 2://лево
case 3://верх

Если убрать комментарии, то становится непонятно, какая цифра за какое направление отвечает.
Так лучше:
private static const RIGHT:int = 0;
private static const LEFT:int = 1;
private static const UP:int = 2;

case RIGHT:
case LEFT:
case UP:


Оптимизация
Заменяем switch на if. If работает быстрее, чем switch. Читается хуже, но работает быстрее.

Рефакторинг
Заносим магическое значение 80, встречающееся ни один раз, в константу. Назовем её PADDING. Забегая вперед, скажу, что я переименовал Main.sWidth и Main.sHeight в Main.stageWidth и Main.stageHeight соответственно.

Функция, отвечающая за расчёт новой координаты:
private function setPosition(point:Point):void 
{
    var direction:int = XORRandom.randomRangeInt(0, 3);
    if (direction == LEFT)
    {
        point.x = -PADDING;
        point.y = XORRandom.random * Main.stageHeight;
        return;
    }
    if (direction == RIGHT)
    {
        point.x = Main.stageWidth + PADDING;
        point.y = XORRandom.random * Main.stageHeight;
        return;
    }
    if (direction == UP)
    {
        point.x = XORRandom.random * Main.stageWidth;
        point.y = -PADDING;
        return;
    }
    if (direction == DOWN)
    {
        point.x = XORRandom.random * Main.stageWidth;
        point.y = Main.stageHeight + PADDING;
        return;
    }
}


Правим немного ещё некоторые мелочи, получаем код:
package objects 
{
    import flash.display.BitmapData;
    import flash.display.Graphics;
    import flash.display.Shape;
    import flash.geom.Point;
    import flash.geom.Rectangle;
    import utils.XORRandom;
    
    public class Circle
    {
        private static const MIN_RADIUS:int = 1;
        private static const MAX_RIADIUS:int = 10;
        
        private static const MIN_SPEED:int = 1;
        private static const MAX_SPEED:int = 5;
        
        private static const RIGHT:int = 0;
        private static const LEFT:int = 1;
        private static const UP:int = 2;
        private static const DOWN:int = 3;
        
        private static const PADDING:int = 0;
        
        // Данные для отрисовки в BitmapData
        public var bitmapData:BitmapData;
        public var rectangle:Rectangle;
        public var point:Point = new Point();
        
        private var destination:Point = new Point();
        private var stepX:Number;
        private var stepY:Number;
        
        public function Circle() 
        {
            init();
        }
        
        private function init():void 
        {
            var radius:int = XORRandom.randomRangeInt(MIN_RADIUS, MAX_RIADIUS);
            var size:int = radius * 2;
            var shape:Shape = getCircle(radius);
            bitmapData = new BitmapData(size, size, true, 0);
            bitmapData.draw(shape);
            rectangle = new Rectangle(0, 0, size, size);
            setPosition(point);
            setNewDestination();
        }
        
        private function getCircle(radius:int):Shape
        {
            var circle:Shape = new Shape();
            var graphics:Graphics = circle.graphics;
            graphics.beginFill(randomColor);
            graphics.drawCircle(radius, radius, radius);
            graphics.endFill();
            return circle;
        }
        
        private function get randomColor():uint
        {
            return XORRandom.random * 0x1000000;
        }
        
        private function setNewDestination():void 
        {
            setPosition(destination);
            calculateStep();
        }
        
        private function setPosition(point:Point):void 
        {
            var direction:int = XORRandom.randomRangeInt(0, 3);
            if (direction == LEFT)
            {
                point.x = -PADDING;
                point.y = XORRandom.random * Main.stageHeight;
                return;
            }
            if (direction == RIGHT)
            {
                point.x = Main.stageWidth + PADDING;
                point.y = XORRandom.random * Main.stageHeight;
                return;
            }
            if (direction == UP)
            {
                point.x = XORRandom.random * Main.stageWidth;
                point.y = -PADDING;
                return;
            }
            if (direction == DOWN)
            {
                point.x = XORRandom.random * Main.stageWidth;
                point.y = Main.stageHeight + PADDING;
                return;
            }
        }
        
        private function calculateStep():void 
        {
            var speed:Number = XORRandom.randomRangeNumber(MIN_SPEED, MAX_SPEED);
            var distance:Number = Point.distance(point, destination);
            var numSteps:int = int(distance / speed);
            var distanceX:Number = destination.x - point.x;
            var distanceY:Number = destination.y - point.y;
            if (numSteps == 0)
            {
                stepX = 0;
                stepY = 0;
            }
            else
            {
                stepX = distanceX / numSteps;
                stepY = distanceY / numSteps;
            }
        }
        
        public function move():void
        {
            if (Point.distance(point, destination) <= MAX_SPEED)
                setNewDestination();
            point.x += stepX;
            point.y += stepY;
        }
    }
}


Main.as

Рефакторинг
Два участка кода повторяются. В функцию их.
sWidth = stage.stageWidth;
sHeight = stage.stageHeight;
field = new BitmapData(sWidth, sHeight, true, 0);
fieldBMP = new Bitmap(field);
stageRect = new Rectangle(0, 0, sWidth, sHeight);


Рефакторинг
Обращаемся к Coding convintions. Массив именуется множественным числом, то есть будет cubes вместо aCube. Так даже код cubes[1] выглядит логичнее, мол, дай мне из кубов тот, что с индексом один. Так как у нас круги, назовем массив circles.

Рефакторинг
Количество кругов занесем в константу N.

Оптимизация
При перерисовке кругов в BitmapData не используется функции lock() и unlock(). Исправляем это.

Рефакторинг
Ещё я поменял названия переменных. В этом не было необходимости, это уже мой загон. Также удалил все комментарии, ни к чему они там.
Вот, что получилось:
package 
{
    import flash.display.Bitmap;
    import flash.display.BitmapData;
    import flash.display.Sprite;
    import flash.display.StageAlign;
    import flash.display.StageScaleMode;
    import flash.events.Event;
    import flash.geom.Rectangle;
    import objects.Circle;
    import utils.Stats;

    public class Main extends Sprite 
    {
        private static const N:int = 6000;
        
        public static var stageWidth:Number;
        public static var stageHeight:Number;
        
        private var bitmapData:BitmapData;
        private var bitmap:Bitmap;
        private var rectangle:Rectangle = new Rectangle();
        private var circles:Vector.<Circle> = new Vector.<Circle>();
        
        public function Main():void 
        {
            if (stage) init();
            else addEventListener(Event.ADDED_TO_STAGE, addedToStageListener);
        }
        
        private function addedToStageListener(e:Event):void 
        {
            removeEventListener(Event.ADDED_TO_STAGE, addedToStageListener);
            init();
        }
                
        private function init():void 
        {
            setStageParameters();
            createBitmap()
            createCircles();
            addChild(new Stats());
            addEventListener(Event.ENTER_FRAME, enterFrameListener);
        }
        
        private function setStageParameters():void 
        {
            stage.align = StageAlign.TOP_LEFT;
            stage.scaleMode = StageScaleMode.NO_SCALE;
            stage.addEventListener(Event.RESIZE, resizeListener);
        }
        
        private function resizeListener(e:Event):void 
        {
            removeChild(bitmap);
            createBitmap();
        }
        
        private function createBitmap():void 
        {
            stageWidth = stage.stageWidth;
            stageHeight = stage.stageHeight;
            bitmapData = new BitmapData(stageWidth, stageHeight, true, 0x00000000);
            bitmap = new Bitmap(bitmapData);
            rectangle.width = stageWidth;
            rectangle.height = stageHeight;
            addChildAt(bitmap, 0);
        }
        
        private function createCircles():void 
        {
            for (var i:int = 0; i < N; i++) 
                circles.push(new Circle());
        }
        
        private function enterFrameListener(e:Event):void 
        {
            var circle:Circle;
            bitmapData.lock();
            bitmapData.fillRect(rectangle, 0x00000000);
            for (var i:int = 0; i < N; i++) 
            {
                circle = circles[i];
                circle.move();
                bitmapData.copyPixels(circle.bitmapData, circle.rectangle, circle.point, null, null, true);
                //var matrix:Matrix = new Matrix(1, 0, 0, 1, circle.point.x, circle.point.y);
                //bitmapData.draw(circle.bitmapData, matrix, null, null, null, true);
            }
            bitmapData.unlock();
        }
    }
}


Результат

В результате проделанных операций код стал гораздо более понятным, а производительность выросла на 20-30%.
Чтобы сравнить до и после, предлагаю вам две флешки. Я немного модифицировал их, теперь можно менять количество кругов стрелками ? и ?.
До | После
Исходный код

Заключение

В заключении я не буду писать никаких нотаций о том, что надо писать правильно и всегда улучшать свой код, просто здоровья вам успешных проектов.
  • +17

Комментарии (18)

0
у меня смешанные чувства от прочтения этой статьи, с одной стороны очень хорошо подошли к рефакторингу и оптимизации, а с другой не заметили что эта флешка жрет память как бегемот. Зачем ЗАЧЕМ при создании каждого объекта создавать ему битмапдату??? не проще создать да хоть 1000 разных шариков-битмапдат, засунуть их всех в массив, а потом объектам раздавать ссылки рандомом на эти шарики в массиве..?
0
и опять же список работал бы бысрее
0
и еще косяк, шарики подходя к краю могут начать ходить вдоль рамки
0
А вообще спасибо, за замечания. Такой алгоритм, я его не менял. Можно поставить параметр PADDING = 80; и тогда, как в оригинале, те круги, что двигаются вдоль рамки не будет видно =).
0
Список бы работал быстрее. Только не надо уходить в крайности. Тут не цикл на миллион итераций. Несколько наносекунд особой роли не сыграют.
0
Тут все шарики уникальны, у каждого свой цвет и размер. Слишком много разных вариантов, все в массив не засунешь. Если бы я отрисовывал тайлы похожим способом, то я бы сначала создал для каждого вида тайла свою BitmapData, а потом, в зависимости от количества объектов эти данные копировал. То есть BitmapData была бы одна, а рисовалась много раз. Но с кругами я не вижу способа, как поступить иначе, чем сейчас.
0
все по-честному — вряд ли попадется пара одинаковых :)
+1
На 5000 объектов в обоих случаях показывает 24 фпс. И для чистоты эксперимента нужно рандомность исключить, размер битмапы влияет на скорость ее отрисовки.
+2
Казалось бы, вроде такие мелочи на которые многие не обращают внимание: «ай, потом поправлю, ай потом оптимизирую, ой да кто будет смотреть на мой код...» — и в итоге получается «избушка на курьих ножках» которая вот вот завалится. А если сразу внимательно подходить к оформлению кода и не оставлять мелочи на потом, то и код читать приятнее самому и у многих мелких багов просто не будет шансов сломать программу. В общем, правильный наглядный пример отношения к коду! :) Плюсую.

ЗЫ: Кстати, я оригинальные исходники так и не дочитал, потому что голова начала пухнуть от запутанного оформления.
0
Начал опять писать про скорость, потом стер :)
  • ryzed
  • ryzed
0
fail

PS: Впервые в жизни у меня FP отъел 1,5 Gb оперативки! :D
0
Когда увидел, удивился. Проверил код, обнаружил глупую ошибку.
Надо.
private function addCircles():void
{
    n += STEP;
    for (var i:int = 0; i < n; i++) 
        circles.push(new Circle());
    textField.text = n.toString();
}

Заменить на:
private function addCircles():void

{
    n += STEP;
    for (var i:int = 0; i < STEP; i++) 
        circles.push(new Circle());
    textField.text = n.toString();
}

Обновил флешку и исходники.
0
вот куда уходила вся память, тогда первый комент теряет в актуальности половину..)
0
Чему равна переменная STEP?
0
Плюсую. Перенести в раздел «ActionScript» и вынести на главную.
0
У вас шарикик все дергаются — и мне так кажется что от bitmap lock()
0
bitmap.lock() не дает обновляться классу Bitmap, пока с BitmapData происходят изменения. Дергаться они могут потому, что координаты шарикам задаются только целые, поэтому и происходит движение «лесенкой». Каких-то других дерганий я не наблюдаю, проверял на трех компьютерах.
0
Дергаются (отнюдь не от округления) каждые 3-4ю секунды — причем дергаются под Виндой ФФ гораздо больше чем под маком ФФ. Вы не подумайте что это ваш код косячит — на форуме уже устали обсуждать эту проблему — просто у оригинальной флешки этих дерганий нет вот это и интересно. А вы рендерите обчным методом и ничего волшебного тут нет — все так делают и у всех дергается. Причем (только FYI) пытаются урезать фпс чтобы хоть както сгладить.
Только зарегистрированные и авторизованные пользователи могут оставлять комментарии.