Betere performance voor pChart

Door ACM op dinsdag 20 oktober 2009 19:10 - Reacties (22)
Categorie: Pricewatch, Views: 4.752

Met het Pricewatch 3.0-project hebben we ook nieuwe grafiekjes geintroduceerd. Op onze schaal, zowel kwa data als aantallen grafieken, is de performance van zelfs dat soort details redelijk belangrijk.

Mijn eerste implementatie van een grafiekje met pChart-omgeving bleek 3 seconde nodig te hebben op onze testserver om een csv-bestand van 300 regels (met ieder een label en 3 datapunten) te vertalen naar een grafiekje. Nadere inspectie met xdebug gaf aan dat een groot deel van die tijd zat in de opslag van data in het pData-object.
Deze heeft een vrij inefficiente interne datastructuur, maar heeft ook nog eens exponentiele tijd nodig om datapunten toe te voegen. Effectief werd er voor elk nieuw datapunt (n+1) n maal geteld hoeveel elementen er al in de array zaten. Om 3x 300 datapunten toe te voegen ben je dan dus aardig wat count-operaties verder :X Als ik het goed uitrekenen zit dat op 3* 45150 count-operaties. Domweg het buiten de loop verplaatsen van de count-operatie leverde al een volle seconde tijdswinst op.
Het vervangen van de behoorlijk inefficiente loops door een simpele toevoeging van een array per datarij leverde nog wat winst op.

Verder stikt de code van loze dubbele if's, zoals dit soort dingen:

PHP:
1
2
3
4
5
6
7
8
if ( $DataDescription["Format"]["Y"] == "time" )
         $Value = $this->ToTime($Value);
if ( $DataDescription["Format"]["Y"] == "date" )
         $Value = $this->ToDate($Value);
if ( $DataDescription["Format"]["Y"] == "metric" )
         $Value = $this->ToMetric($Value);
if ( $DataDescription["Format"]["Y"] == "currency" )
         $Value = $this->ToCurrency($Value);


Als je weet dat die waarde "time" heeft, hoef je de andere if's niet meer uit te voeren... In dit geval had de code zelfs nog vervangen kunnen worden door een switch-case structuur, maar ik vond de toevoeging van else's voldoende.

Ook stikte de code van de duplicate floor-calls, waar in deze context een cast naar int sowieso al voldoende was. Voor elke alfatransparante die getekend moest worden, heb ik deze niet zo efficiente constructie vervangen:

PHP:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
$Xi   = floor($X);
     $Yi   = floor($Y);
 
      if ( $Xi == $X && $Yi == $Y)
       {
// ...
       }
      else
       {
       $Alpha1 = (((1 - ($X - floor($X))) * (1 - ($Y - floor($Y))) * 100) / 100) * $Alpha;
        if ( $Alpha1 > $this->AntialiasQuality ) { $this->drawAlphaPixel($Xi,$Yi,$Alpha1,$R,$G,$B); }
 
       $Alpha2 = ((($X - floor($X)) * (1 - ($Y - floor($Y))) * 100) / 100) * $Alpha;
        if ( $Alpha2 > $this->AntialiasQuality ) { $this->drawAlphaPixel($Xi+1,$Yi,$Alpha2,$R,$G,$B); }
 
       $Alpha3 = (((1 - ($X - floor($X))) * ($Y - floor($Y)) * 100) / 100) * $Alpha;
        if ( $Alpha3 > $this->AntialiasQuality ) { $this->drawAlphaPixel($Xi,$Yi+1,$Alpha3,$R,$G,$B); }
 
       $Alpha4 = ((($X - floor($X)) * ($Y - floor($Y)) * 100) / 100) * $Alpha;
        if ( $Alpha4 > $this->AntialiasQuality ) { $this->drawAlphaPixel($Xi+1,$Yi+1,$Alpha4,$R,$G,$B); }
       }
     }

// Vervangen door:
     $Xi   = (int)$X;
     $Yi   = (int)$Y;
 
      if ( $Xi == $X && $Yi == $Y)
       {
// ..
       }
      else
       {
        $xdiff = ($X - $Xi);
        $ydiff = ($Y - $Yi);
       $Alpha1 = (((1 - $xdiff) * (1 - $ydiff) * 100) / 100) * $Alpha;
        if ( $Alpha1 > $this->AntialiasQuality ) { $this->drawAlphaPixel($Xi,$Yi,$Alpha1,$R,$G,$B); }
 
       $Alpha2 = (($xdiff * (1 - $ydiff) * 100) / 100) * $Alpha;
        if ( $Alpha2 > $this->AntialiasQuality ) { $this->drawAlphaPixel($Xi+1,$Yi,$Alpha2,$R,$G,$B); }
 
       $Alpha3 = (((1 - $xdiff) * $ydiff * 100) / 100) * $Alpha;
        if ( $Alpha3 > $this->AntialiasQuality ) { $this->drawAlphaPixel($Xi,$Yi+1,$Alpha3,$R,$G,$B); }
 
       $Alpha4 = (($xdiff * $ydiff * 100) / 100) * $Alpha;
        if ( $Alpha4 > $this->AntialiasQuality ) { $this->drawAlphaPixel($Xi+1,$Yi+1,$Alpha4,$R,$G,$B); }
       }
     }



Het eindresultaat van de wijzigingen vindt je in deze patch, let er daarbij op dat ik de .class-files hernoemd heb naar .class.php. Verder zou ie op pChart 1.27d moeten werken. Aangezien we .class als extentie al voor java's class files hebben gereserveerd maakte dat het editten van die files in eclipse met pdt zo lastig :)

Het eindresultaat was dat de grafiek op de vrij langzame testserver minder dan een seconde nodig had... Het kan vast nog steeds wel wat efficienter, maar de ergste knelpunten zijn er voor ons nu in ieder geval uit.

Volgende: T.net browserstatistieken 2007 - 2009 12-'09 T.net browserstatistieken 2007 - 2009
Volgende: T.net browser-statistieken juni 2009 07-'09 T.net browser-statistieken juni 2009

Reacties


Door Tweakers user ThaStealth, dinsdag 20 oktober 2009 19:54

Ik snap niet waarom jullie niet zelf een lichtgewicht component maken voor het renderen van de grafieken. In het stukje code wat je toont zitten nog een aantal zaken in die er naar mijn inziens uitgesloopt kunnen worden en dus weer een tijdswinst oplevert.

3*300 punten is voor een machine geen probleem normaal gezien, en moet toch binnen een extreem korte tijd afgewerkt zijn. Ik weet niet precies wat de huidige rendertijd is, maar zoals ik je verhaal leer (en wat je al kan bereiken met een paar simpele optimalisaties), lijkt het mij dat er nog winst te boeken is

Door Tweakers user anthonycd, dinsdag 20 oktober 2009 20:05

Je patch lijkt mij interessant, maar waarmee open ik een .patch bestand onder Windows? Renamen naar .zip werkt niet :P

Door Tweakers user ACM, dinsdag 20 oktober 2009 20:11

ThaStealth, een nette omgeving om universeel grafieken met anti-aliasing maken is nou niet bepaald iets dat je "even" doet. Om die reden hebben we er voor gekozen om pChart te gebruiken en dan maar voor lief te nemen dat er wat inefficiente code in zit. De ergste inefficienties heb ik er nu uitgehaald. Wat er nu aan tijd over is, is vooral het simpele feit dat ie voor de anti-aliased datapunten uiteindelijk steeds 5 pixels tekent.
Dat kan vast ook nog wel wat efficienter, maar dan moet je weer meer data gaan bijhouden, uitgebreid profilen etc... Niet iets waar we een enorme hoeveelheid tijd aan kunnen besteden, gezien het in het groter toch relatief kleine belang van de plaatjes (ze worden per stuk sowieso maar een keer per dag gemaakt).

anthonycd: patch is gewoon platte tekst, ik gok dat een tool als winmerge die sowieso redelijk kan toepassen op een bestaande dir/file.

[Reactie gewijzigd op dinsdag 20 oktober 2009 20:11]


Door Tweakers user gamer13, dinsdag 20 oktober 2009 20:14

Je patch lijkt mij interessant, maar waarmee open ik een .patch bestand onder Windows? Renamen naar .zip werkt niet
Dat kan met allerlei programma's. Momenteel heb ik Tortoise SVN geinstalleerd en die bevat een 'Diff' tool, waarmee je een patch file kan bekijken.

Door Tweakers user anthonycd, dinsdag 20 oktober 2009 20:16

met notepad/wordpad etc. kan ik hem inlezen, even kijken hoe ik hem kan toepassen ipv alles handmatig copy-pasten...

TortoiseSVN did the trick. Thanks allebei.

[Reactie gewijzigd op dinsdag 20 oktober 2009 20:29]


Door Tweakers user Maxxi, dinsdag 20 oktober 2009 20:32

Leuk om te lezen, maar wat zijn "duplicate floor-calls"?

Door Tweakers user ACM, dinsdag 20 oktober 2009 20:36

't zijn duplicate calls, herhalingen, naar floor. In het 2e voorbeeld zie je dat er flink wat floor($X)-calls gedaan worden, terwijl dat ook een keer in en variabele had gekund en dan die variabel te gebruiken.

Door Tweakers user anthonycd, dinsdag 20 oktober 2009 20:49

Ben in de patch nog een foutje tegengekomen:

regel 352:

code:
1
+           $this->Data[$nextId]['Name''] = $nextId;


moet zijn

code:
1
+           $this->Data[$nextId]['Name'] = $nextId;



denk ik

[Reactie gewijzigd op dinsdag 20 oktober 2009 20:50]


Door Tweakers user mzziol, dinsdag 20 oktober 2009 21:29

matplotlib i.c.m. Cairo is voor mij altijd nog heel snel geweest. Misschien een ideetje?

Door Tweakers user afraca, dinsdag 20 oktober 2009 21:35

Nu hoop ik uiteraard dat dergelijke ontwikkelingen ook worden doorgevoerd naar pchart zelf, en dat iedereen ervan kan profiteren. Het fijne van open source.

(was onder de indruk van pchart, nog steeds, maar dit is wel enigszins een tegenvaller)

Door Tweakers user ACM, dinsdag 20 oktober 2009 22:33

Klopt anthonycd, gelijk even aangepast.

Door Tweakers user crisp, dinsdag 20 oktober 2009 22:40

Er zitten echt bizarre dingen in die class:


PHP:
1
2
3
4
5
6
7
8
9
$aPoints   = "";
$aPoints[] = $XLast;
$aPoints[] = $YLast;
$aPoints[] = $XPos;
$aPoints[] = $YPos;
$aPoints[] = $XPos;
$aPoints[] = $YZero;
$aPoints[] = $XLast;
$aPoints[] = $YZero;


waarom niet meteen:

PHP:
1
2
3
4
5
6
7
8
9
10
$aPoints = array(
    $XLast,
    $YLast,
    $XPos,
    $YPos,
    $XPos,
    $YZero,
    $XLast,
    $YZero  
);

:?

of dit:

PHP:
1
2
3
4
5
6
7
8
9
$ID = 0;
$count = count($this->Data);
for($i=0;$i<=$count;$i++)
{
    if(isset($this->Data[$i][$Serie]))
    {
        $ID = $i+1;
    }
}


Begin dan ook gewoon achteraan:

PHP:
1
2
3
4
5
6
7
8
9
10
$ID = 0;
$i = count($this->Data);
while($i--)
{
    if(isset($this->Data[$i][$Serie]))
    {
        $ID = $i+1;
        break;
    }
}



Het barst echt van dit soort vaagheden 8)7

Door Tweakers user ACM, dinsdag 20 oktober 2009 22:48

Er zitten wel meer lelijke dingen in, zoals al die array-initialisaties met lege string in, als ik ooit tijd heb zou ik de code kunnen herschrijven zodat ie uiteindelijk hetzelfde doet, maar dan met correctere PHP... Anderzijds zijn er ook wel wat projecten waarbij men dat al gedaan heeft of doet, dus wellicht is het een kwestie van afwachten en over een tijdje pChart vervangen door een library die dezelfde output combineert met nettere code :)

[Reactie gewijzigd op dinsdag 20 oktober 2009 22:48]


Door Tweakers user crisp, dinsdag 20 oktober 2009 23:02

Tsja, mtChart (fork van pChart) bevat ook nog steeds veel vaagheden/inefficiŽnties van pChart. Er lijkt echter wel meer activiteit te zijn op dat project.

Door Tweakers user RobIII, woensdag 21 oktober 2009 00:26

Ah, the good-old Schlemiel the Painter's algo :P (Wiki)

[Reactie gewijzigd op woensdag 21 oktober 2009 00:36]


Door Tweakers user YopY, woensdag 21 oktober 2009 09:20

Lijkt mij een geval van copy-paste code (dat tweede stukje met die floors()), en/of zoiets van 'het werkt en ifs zijn niet zo zwaar dus laten we het zo'. Dit soort dingen worden vaak vergeten over tijd of in de achtergrond gezet, omdat er vaak meer gepushed wordt naar nieuwe functionaliteit. Of het is een respect dingetje - dit soort code zat er al vanaf het begin in, dat raken we niet aan want misschien wordt er iemand kwaad of maken we het stuq.

Of ik zie het verkeerd, :+.
quote: robIII
Ah, the good-old Schlemiel the Painter's algo (Wiki)
Goed artikel, die site moet ik echt vaker bekijken :+.

[Reactie gewijzigd op woensdag 21 oktober 2009 09:27]


Door Tweakers user ZpAz, woensdag 21 oktober 2009 10:14

Wat is eigenlijk de reden dat jullie voor een php (gd) oplossing hebben gekozen, ipv voor een chart welke clientside (javascript) wordt getekend? Dan kan je ook nog meteen spelen met 'hovers' e.d :)

Zelf ben ik wel van fan flot (jQuery gebasseerd systeem).
http://people.iola.dk/olau/flot/examples/

Werkt vanaf IE6. Je moet dan wel JS ingeschakeld hebben dat is waar.

Door Tweakers user ACM, woensdag 21 oktober 2009 10:19

We vinden het niet zo'n prettig idee om dat soort dingen van client-side support af te laten hangen. Vooral de flash-versies vonden we wat dat betreft geen goed idee.

Bovendien moet er alsnog vrij veel data doorgestuurd worden naar de javascript-library, dus is het ook nog maar de vraag hoe goed dat in mobiele devices met zwakke processors - maar wel javascript - presteert.

Door Tweakers user ZpAz, woensdag 21 oktober 2009 12:06

Ja precies, aan de mobiele devices had ik niet gedacht.

Door Tweakers user Tharulerz, vrijdag 23 oktober 2009 18:34

Kijk eens naar amcharts? (amcharts.com)

Ok, je gaat waarschijnlijk niet ineens overstappen naar een andere tool, maar deze eet gwn xml, en zen snelheid hangt dus enkel af van hoe snel jou script de xml aanmaakt :)

Edit: Uiteraard heeft amcharts een export naar image functie zodat je ook mobile devices kan tevreden stellen

[Reactie gewijzigd op vrijdag 23 oktober 2009 18:35]


Door Tweakers user JeRa, donderdag 24 juni 2010 00:09

Patch even uitgeprobeerd, een grafiek die voorheen 5 seconden duurde om te renderen doet dat nu in ťťn. Erg netjes dus. En zoals hier al gezegd wordt, het kan allemaal nog veel sneller :)

In de patch wordt ook functionaliteit m.b.t. zogenaamde X-ticks toegevoegd. Ik zag dit niet terug in de grafiek; wat doet dit precies?

[Reactie gewijzigd op donderdag 24 juni 2010 00:09]


Door Tweakers user ACM, donderdag 24 juni 2010 08:21

JeRa schreef op donderdag 24 juni 2010 @ 00:09:
In de patch wordt ook functionaliteit m.b.t. zogenaamde X-ticks toegevoegd. Ik zag dit niet terug in de grafiek; wat doet dit precies?
Dat zorgt er voor dat er wel verticale lijntjes getekend kunnen worden, maar dan alleen bij de tickmarks, ipv bij veel meer punten wat het standaard gedrag van pChart is. Met de hoeveelheid x-punten die wij hebben leverde pChart's standaard gedrag effectief gewoon een grijs blok op de achtergrond op.

Reageren is niet meer mogelijk