poniedziałek, 16 lipca 2012

Dlaczego ważne jest by nie używać ponownie tych samych zmiennych lokalnych

W tym poście chciałbym pokazać do czego prowadzi pewna "dziadowska oszczędność" czyli mikrooptymalizacja w kodzie.

W ostatnim poście, opisałem niektóre zalecane (o zgrozo) mikrooptymalizacje które mają uczynić twój kod nieczytelnym i przy okazji zwiększyć jego wydajność (autor zapomniał dopisać że będzie to rzędu 3-4%).

Właśnie przed chwilą szukałem dość poważnego błędu w dość starym kawałku mojego kodu (połowa węzłów drzewa miała wartość null, zamiast liczby nieparzystej).


    /**
     * Metoda rekurencyjnie zaglebia sie w drzewo i tworzy strukture drzewiastą o zadanym stopniu głębokosci, wywolujemy ja
     * dla kazdego węzła poddrzewa
     *
     * @param int      $level            poziom głębokosci drzewa, gdy dojdziemy do 1 to Metoda sie zatrzymuje
     * @param bool     $insertNullFields Czy Metoda na tym poziomie ma wstawic puste miejsce na nowego przegranego czy nie
     * @param TreeNode $node             galaz dla ktorej bedziemy przeprowadzac operacje
     */
    protected function createChildNode($maxLevel, $insertNullFields, TreeNode $node, $currentLevel,
                                       $numeratorTableCounter = null, $roundID = null)
    {
        if ($roundID < 10 || $roundID == null) {
            echo "błąd";
        }

        if ($currentLevel == $maxLevel) {
            /* Dla obydwu dzieci tego węzła ktore beda ostatnimi węzłami w tym drzewie */
            for ($counter = 0; $counter < 2; $counter++) {
                /* Przypisz dla obecnie edytowanego węzła nowy węzeł jako dziecko, ID weź z numeratora poniewaz są
        * to puste węzły z miejscem na przegranych odpadajacych z turnieju, algorytm sie zatrzymuje,
        * dla dzieci dalej nie robimy juz nic */
                $newNode = TreeNode::create($this->nodes, $node, null, null, null, $roundID);
                /* Przypisz ten wezeł jako pojemnik na przegranych na odpowiednim poziomie i pozycji */
                $this->numeratorTable[$numeratorTableCounter][] = $newNode->ID;
            }
        } else {
            if ($currentLevel < $maxLevel) {
                /* Wstawiamy jedno puste pole i jedno sie dalej rozgałęzia, uruchomiono Metode z atrybutem True */
                if ($insertNullFields) {
                    /* Puste pole */
                    $newNode = TreeNode::create($this->nodes, $node, null, null, null, $roundID);
                    $node->children[0] = $newNode;
                    /* Wpisujemy do tabelki z polami przegranych */
                    $this->numeratorTable[$numeratorTableCounter][] = $newNode->ID;
                    /* Pole rozgaleziajace sie dalej (miejsce na bitwe i na jej wynik) */
                    $newNode = TreeNode::create($this->nodes, $node);
                    /* I dla drugiego dziecka wywolujemy rekurencyjnie tą Metode, tym razem z atrybutem "false", poziom zmniejszamy */
                    $this->createChildNode($maxLevel, false, $newNode, ($currentLevel + 1),
                        ($numeratorTableCounter + 1), ($roundID - 1));
                } else {
                    /* Metoda uruchomiona z atrybutem false (wstaw 2 galezie i im dzieciom tez uruchom ta Metode) */
                    for ($counter = 0; $counter < 2; $counter++) {
                        $newNode = TreeNode::create($this->nodes, $node, null, null, null, $roundID);
                        /* uruchamiamy tą Metode dla nowo wstawionego dziecka, z atrybutem true (wstaw jedno pole pełne i jedno puste) */
                        $this->createChildNode($maxLevel, true, $newNode, ($currentLevel + 1),
                            ($numeratorTableCounter), ($roundID - 1));
                    }
                }
            }
        }
    }
ok, to jest kod całej metody, nie będziemy się zagłębiać w to czy jest dobrze napisana, bo wiedza zgodnie z prawem moora rośnie wykładniczo z czasem, co daje taki efekt, że człowiek nie chce patrzeć na kod który pisał rok temu bo uważa go za shit.

oto kawałek który był błędny
//[...]
 if ($currentLevel < $maxLevel) {
                /* Wstawiamy jedno puste pole i jedno sie dalej rozgałęzia, uruchomiono Metode z atrybutem True */
                if ($insertNullFields) {
                    /* Puste pole */
                    $newNode = TreeNode::create($this->nodes, $node, null, null, null, $roundID);
                    $node->children[0] = $newNode;
                    /* Wpisujemy do tabelki z polami przegranych */
                    $this->numeratorTable[$numeratorTableCounter][] = $newNode->ID;
                    /* Pole rozgaleziajace sie dalej (miejsce na bitwe i na jej wynik) */
// TUTAJ JEST BŁĄD - powinno być w parametrze jeszcze null,null, $roundID
                    $newNode = TreeNode::create($this->nodes, $node);
                    /* I dla drugiego dziecka wywolujemy rekurencyjnie tą Metode, tym razem z atrybutem "false", poziom zmniejszamy */
                    $this->createChildNode($maxLevel, false, $newNode, ($currentLevel + 1),
                        ($numeratorTableCounter + 1), ($roundID - 1));
                }
//[...]


jak widać mamy tutaj zmienną newNode użytą w dwóch miejscach.

pierwsze użycie to

$newNode = TreeNode::create($this->nodes, $node, null, null, null, $roundID);
natomiast drugie to

$newNode = TreeNode::create($this->nodes, $node); // notabene ta druga również powinna mieć jeszcze 3 parametry: null,null, $roundID

W pierwszym momencie, pomyślałem sobie, (to dośc logiczny wniosek) że, po prostu omyłkowo mi się skopiował kod z pierwszego wywołania w miejsce drugiego, tylko jeszcze do tego wcieło mi 3 parametry i obiekt dobry zostal zastąpiony złym. W końcu jest to zrobione do jednej zmiennej $newNode.

Dopiero wnikliwsza analiza pozwoliła mi stwierdzić, że tak naprawde to powinny być utworzone dwie TreeNode, jedna na góre i jedna na dół. Pierwsza jest tworzona w $treeNode, zostaje dodana do drzewa, następnie druga TreeNode zostaje przypisana do $treeNode i również dodana do drzewa. Jednak te obiekty powinny się one znajdować w dwóch zmiennych - $upNewNode i $downNewNode, wtedy po spojrzeniu na metodę od razu bym wiedział że są to dwa rozdzielne obiekty, a nie jeden ciągły.

Dlatego nie powinno się "reużywać" starych lokalnych zmiennych dla nowych obiektów - jeżeli chcemy pisać kod poprawny semantycznie i taki który da się rozszyfrować nawet jeśli nie ma komentarzy - a komentarze z kodu chcemy w miare mozliwości usuwać i zastępować je czytelnym kodem - podwójnie użyta zmienna nie pozwala na odczytanie jej sensu z samego kodu. Nawet jeśli pierwszy obiekt jest uzywany w pierwszej części metody, a drugi w drugim. Panowie i panie! Robimy dwie zmienne, nie oszczędzamy na 2 bajtach pamięci bo później są z tego tytułu same problemy.

piątek, 6 lipca 2012

Google Code Optimizations: Avoid getters and setters

Wielki szacun dla Google. Jest to świetna, i bardzo innowacyjna firma, ich produkty również zazwyczaj były bardzo dobre, i co ważne działały w różnych regionach świata, czego czasem nie można było powiedzieć o konkurencji. Od strony technicznej raczej rzadko można było się do czegoś przyczepić, google nie sypało critical errorami ani nie znikało z netu. Aby to osiągnąć, google (taki jest mój obecny stan wiedzy na ten temat) korzystało z mieszanki pythona, C, C++, i chyba jeszcze jakiegoś języka. W każdym razie, nie było tam PHP.

I oto google wypuszcza nam artykuł, jak optymalizować skrypty PHP, by wykonywały się szybciej.

Optimizing PHP scripts speed with Google

Do pierwszych kilku porad nic nie mam. Profilowanie kodu, memcache (gdy rozwiążemy problem konkurencji w dostępie do cache uruchamianych w wielu wątkach skryptów PHP), może znacząco podnieśc wydajność.

I oto trafiamy na najlepszą poradę
Avoid writing naive setters and getters When writing classes in PHP, you can save time and speed up your scripts by working with object properties directly, rather than writing naive setters and getters. In the following example, the dog class uses the setName() and getName() methods for accessing the name property.

Setting and calling the name property directly can run up to 100% faster, as well as cutting down on development time.


co za gniot. postaram się wykazać jego bezsensowność.

Załóżmy że robimy klasę. Prostą klasę, ma dwa pola, z jakichś powodów nie można ich ustawić w konstruktorze.

Podążąjąc za paradygmatem OOP w którym pole klasy jest jego wewnętrznym szczegółem implementacji, i nie powinien być publiczny by w przyszłości móc go zmienić bez łamania API klasy, powinniśmy uczynić te pola prywatnymi, i dorobić do nich akcesory get/set.

Jednak jest to wolne, ble, i w ogóle trzeba się kupe czasu opisać (swoją drogą - czy ten facet z google widział kiedyś na oczy choćby darmowe Eclipse PDT czy Netbeans PHP, który ten kod generuje sam z siebie?), więc nie piszemy akcesorów, zaoszczędzamy procesor, pamięć oraz czas.

public class Example
{
   public $fieldOne;
   public $fieldTwo;
}


Klasa idzie w świat, albo choćby rozpełza się po projekcie, jej API zostaje użyte w kilkudziesięciu miejscach w kodzie. Przypominam że język PHP to język z dynamicznym typowaniem, który jeśli nie posiadamy bardzo dobrego edytora, błędów typów nam po prostu nie wyłapie (zresztą zaawansowane narzędzia czasami również tego nie potrafią niestety zrobić).

Nagle z jakiegos powodu okazuje się że jedno pole powinno być stringiem, a drugie integerem z zakresu 1-100. Co można zrobić w takiej sytuacji?

Mamy dwa wyjścia. Bardziej wydajne (wg Google): Dopisać PHPdoc informujący o tym że jedno z pól jest stringiem, a drugie int. W komentarzu dopisujemy że wartość spoza zakresu 1-100 spowoduje błąd.

Drugie wyjście. Dorobić akcesory i zrobić w nich walidację typu (php nie ma statycznie typowanych zmiennych/pól ale można wymusić typy w metodach, więc akcesory mogą służyć niejako walidacji).

No i teraz mamy problem. I to spory. Trzeba znaleźć wszystkie użycia danej klasy, zamienić $obj->fieldOne, na $obj->getFieldOne() i tak samo z setami. Jestem niemal przekonany że nie znajdziesz wszystkich od razu. Odczytasz sobie nulla z nieistniejącego pola w klasie, i php nawet nie pomyśli żeby cię o tym powiadomić. I będziesz debugował 2 godziny logikę, zastanawiając się dlaczego tablica zwraca -1, wartość która miała być stringiem jest liczbą float w dodatku jest równa minus ileśtam, tylko dlatego że głupie odwołanie do publicznego pola zwróciło ci null zamiast wartości.

Warto było? Mam w swoich kodach pare takich klas które się rozeszły po całym projekcie i mają publiczne pola. I te pola to teraz spory problem, ale nawet nie podejmuje się zamiany ich na akcesory, nie mając kodu pokrytego testami jednostkowymi chociaż w 90%. Po prostu teraz muszę pamiętać że jakiejś zmiennej się nie ustawia na -1 bo to wysypie jakiś program, a inne pole jest tylko do odczytu, chociaż jest publiczne (bez akcesorów nie da się zrobić pól readonly w PHP).

Tak się kończy zabawa w mikrooptymalizacje. Akcesor nie jest taki zły, nawet jeśli robi to samo co publiczne pole. Nigdy nie wiadomo czy będzie taki w przyszłości. Czy w kolejnej wersji programu nie będziesz chciał dodać w tym miejscu lazy-load albo podmienić algorytm na bardziej efektywny. Upubliczniając wewnętrzne szczegóły klasy jakim są pola nie da się tego zrobić bezboleśnie. Klasa nie powinna mieć publicznych pól choćby nie wiem co! Chyba że jest tylko niezbyt inteligentnym DTO, pojemnikiem na dane które muszą mieć swoje nazwy, ale nawet tam gettery i settery mogą się przydać.. być może nie teraz a kiedyś.

Jeśli stosujesz się do konwencji że wszystkie properties klasy które możesz odczytać zaczynasz od get... a wszystko co możesz zapisać od set.. to po wpisaniu do IDE $obj->get otrzymujesz elegancką listę tego co możesz przypisać, analogicznie z set. Mając mix akcesorów i publicznych pól cięzko rozróżnić co jest propertiesami a co zwykłymi metodami.

Oczywiście gettery i settery są złe. Znacznie lepiej jest robić klasy które wykonują czynności wewnątrz siebie, pobierając po prostu parametry, niż udostępniać prywatne pola w pełni publicznymi akcesorami, i manipulować nimi na zewnątrz klasy (choć przy niektórych wzorcach projektowych się tego używa, np visitor).

class Zla
{
   private $predkosc;
   private $odleglosc;
   public function getOdleglosc(){ return $this->odleglosc; }
   public function setOdleglosc($value) { $this->odleglosc=$value; }
   public function getPredkosc() {return $this->predkosc;}
   public function setPredkosc($value) {$this->predkosc=$value;}
}

$zla = new Zla();
$zla->setPredkosc(10);
$zla->setOdleglosc(20);
// itd itp


class Dobra
{
    private $predkosc;
    private $odleglosc;
    public function getOdleglosc(){ return $this->odleglosc;}
    public function getPredkosc() {return $this->predkosc;}

    public function przejedzKawalek($odleglosc, $predkosc)
    {
       // logika jazdy
    }
}

$dobra=new Dobra();
$dobra->przejedzKawalek(10,20);
// itd itp


Jednak należy zwrócić uwagę że tutaj motywacją do usunięcia setterów jest semantyka, a nie pseudooptymalizacje wydajnościowe i lenistwo programisty któremu nie chce sie wcisnąć CTRL+G.

Dodatkowym argumentem za stosowaniem getterow i setterów jest to że IDE łatwiej zrobi rename metod akcesorów, jeśli zmieni się nieco ich znaczenie, niż znajdzie wszystkie miejsca gdzie użyłeś danego pola i zamieni to na mutator.

Zresztą, dbanie o wydajność w trakcie pisania kodu, w takim języku jakim jest PHP (jest to jeden z najwolniejszych języków programowania), zakrawa na ironię. Napisz, tak żeby było czytelnie i bezbłędnie, odpal profiler, popraw jedną metodę która zajmuje 90% czasu skryptu tak żeby było szybko, ciesz się wydajnością. Profilowanie aplikacji ma sens. Mikrooptymalizacje nie. Getter poprawia nieco i tak kulejącą w php kontrolę nad poprawnością kodu. On ci chociaż wywali błąd, przynajmniej dowiesz sie o nim, nie będziesz musiał zgadywac co jest nie tak.
Profilowaniem często można osiągnąć poprawe wydajności znacznie lepszą (i nie zaburzającą czytelności) niż psuciem i zasmiecaniem kodu korzystając ze średniowiecznych technik kodowania. Zresztą, jak chcesz żeby było szybko, to pisz w assemblerze. Po co ci PHP? Assembler zrobi to najszybciej.

W przypadku aplikacji webowych i tak najczęściej wąskim gardłem jest baza danych, więc te pseudooptymalizacje dadzą wymiernie mały efekt przy dużym wysiłku.

Pewnie mało z programistów PHP miało styczność z językiem C#. W tym języku akcesory można deklarować bardzo łatwo
class One
{
    // to jest odpowiednik prostego getPole setPole w javie/PHP
    public int Pole1 {get;set;}
    // w razie potrzeby np. walidacji lub lazy loadu, lub reagowania na zmiane jakiegoś pola
    // możemy rozwinąć powyższy property do postaci poniższej, bez zmieniania interfejsu klasy
    private int _pole2;
    public int Pole2 
    {
       get { return this.doSomethingWithPole2OnGet(); }
       set { this.setSomethingInPole2(value); }
    }
    private int doSomethingWithPole2OnGet()
    {
        // zrob z polem co chcesz zanim zwrocisz. Nie trzeba robic oddzielnej metody
        // ale jest to w C# dobra praktyka by jakies bardziej skomplikowane
        // operacje nie byly w akcesorach tylko w metodach uzywanych w nich.
    }
    private void setSometingInPole2(int value)
    {
        // j.w.
    }
}
Nigdy nie widziałem dyskusji przeciwko getterom i setterom w C#. Ludzie po prostu z nich korzystają. Niby mały lukierek składniowy, mamy do wpisania(wygenerowania) kilka znaków {get;set;} a nie całą litanię getPole/setPole, a ludzie już inaczej zupełnie podchodzą do tego. A może po prostu ludzie piszący w C# mając do dyspozycji szybki język kompilowany, nie muszą poprawiać jego wydajności pseudooptymalizacjami i mogą sie skupic na czytelności, a może po prostu PHP jak lep przyciąga złe praktyki programistyczne? Być może coś w tym jest. Być może się powinno spędzić troche czasu przy javie/C# aby nauczyć się dobrej obiektowości, i dopiero siąść do PHP który ma tą obiektowość momentami wybrakowaną, która sprzyja wyrabianiu złych nawyków.

Nie zajmę stanowiska bo nie chcę flamewara :)

Reasumując - jeśli masz dobry powód do usunięcia gettera/settera z kodu to je usuń, jednak nie rób tego z lenistwa lub dla wydajności. A to jest po prostu piękne.
Don't copy variables for no reason Sometimes PHP novices attempt to make their code "cleaner" by copying predefined variables to variables with shorter names before working with them. What this actually results in is doubled memory consumption (when the variable is altered), and therefore, slow scripts. In the following example, if a user had inserted 512KB worth of characters into a textarea field. This implementation would result in nearly 1MB of memory being used. $description = strip_tags($_POST['description']); echo $description; There's no reason to copy the variable above. You can simply do this operation inline and avoid the extra memory consumption: echo strip_tags($_POST['description']);
No proszę. A ja myślałem że wolne skrypty to takie które nie keszują ciężkich zapytań do bazy. Albo robią 10 razy to samo, gdy wystarczy raz.

Czy ten człowiek zastanawiał się kiedyś dlaczego coś mu w kodzie nie działa, i która tablica w tym przepięknym łańcuchu
   echo getFromDb(intval(strip_tags($_POST['description'.$id.'_'.$fieldName]))+$id);
mu zwraca -1 i dlaczego? Ale to taki piękny i czysty kod, jest idealny, wszystko inline, nie ma żadnych zbędnych zmiennych. Nie wiem jak jemu, mi jest szkoda czasu na wpisywanie po kolei wszystkiego w watch, lub rozbijanie tych wyrażeń na poszczególne zmienne gdy znajde w nich błąd. Ja wole to.
   $fieldName= 'description'.$id.'_'.$fieldName;
   $valueInPost = $_POST[$fieldName];
   $strippedValue = strip_tags($valueInPost);
   $intValueOfStripped = intval($strippedValue);
   $idOfRow = $intValueOfStripped+$id;
   $queryResult = getFromDb($idOfRow);
   echo $queryResult;


Zaraz, zaraz.. czyżbyśmy właśnie przyoszczędzili na komentarzach, które do pierwszego przykładu trzeba by było nieuchronnie napisać?
Kod skomentował się sam. I to jest świetne. Bo gdybym napisał komentarz, jestem prawie pewien ze gdybym coś pozmieniał, to komentarz zostałby niezmieniony i za miesiąc wprowadziłby mnie w błąd.
Ustawię breakpoint na queryResult, uruchomię skrypt, będę miał w IDE podgląd wartości wszystkich zmiennych, szybko znajdę tą która popsuła mi skrypt. Bo raczej we wnętrzu strip_tags nie umieszcze breakpointa...

Zresztą, Zend engine w PHP5 ma pewien mechanizm który powoduje że renundantne przypisanie
    $fieldName2 = $fieldName;
    $fieldName3= $fieldName2;
    doSmth($fieldName3);
nie zużyje pamięci na dodatkowe zmienne $fieldName2 i 3. Także spokojnie można poprawiać czytelnośc kodu dodatkowymi zmiennymi. Zresztą, nawet jakby miało zużywać pamięć to i tak wolę tą czytelną wersję.
Jak już pisałem wcześniej, jeżeli podczas profilowania okaże się że ten kawałek wykonuje sie przez 90% czasu skryptu, to go zoptymalizuję, napisze nieczytelnie ale tak żeby był szybki.
Wcześniej nie.
A tu już bardziej zmyślony przykład, ale takie czasami się zdarzają wewnątrz klas które modelują rzeczywiste procesy i muszą operować na logice
    $is_ended = $tournament->isEnded || ($tournament->state = STATE_LEAGUE_CHECK && $tournament->time > time()) || ($tournament->state = STATE_TOURNAMENT_SIGNIN && $tournament->signedTeams > 30) && ........ ;


 // a ja wolę tak
  $tournamentIsEnded = $tournament->isEnded;
  $tournamentIsInCheckin = $tournament->state = STATE_LEAGUE_CHECK && $tournament->time > time();
  $maxAmountOfTeams = 30;
  $tooMuchSignedTeamsToTournament = $tournament->state = STATE_TOURNAMENT_SIGNIN && $tournament->signedTeams > $maxAmountOfTeams;
  $is_ended= $tournamentIsEnded || $tournamentIsInCheckin || $tooMuchSignedTeamsToTournament ||... ;
Czy nie lepiej sie czyta takie coś? Czy może mam płakać że skrypt mi zajmie 20 kb więcej w pamięci, pomimo że wykona się bezbłędnie?
Jeśli 20kb pamięci robi ci różnicę, to jak już pisałem może czas dokupić nowy serwer, albo napisać ten moduł w C skoro jest taki ważny, jak to robił Facebook zanim napisał HipHopa?
Już w "Clean Code" było jak wół napisane że wyrażenia logiczne są i tak trudne, nie komplikuj ich. Czy tak trudno przenieść zalecenia napisane dla javy, na grunt PHP którego obiektowość jest w dużej mierze skopiowana z javy? Dużo ludzi wpadło na taki pomysł ale ten spec od optymalizacji już nie.

PHP5 jest mocno inspirowany Javą. Java powstała aby zapewnić przenośność kodu w stosunku do C++, oraz, by wyeliminować źródła najczęstszych błędów programistycznych jakie zdarzały się programistom C++, dlatego zawierała szereg usprawnień. PHP upodobniło swoją obiektowość do javowej, więc weźmy przykłąd z nich i piszmy tak by błędów było mniej a nie więcej.
Bo wedle praw murphyego jeśli gdzieś da się popełnić błąd, to błąd zostanie popełniony.

Jeszcze wcześniej google błysneło zajebistą poradą aby zastępować nawiasy
   $query =  "select $fields from $table where $id=$valueOfID";

na konstrukcje
   $query = 'select '.$fields.' from '.$table.' where '.$id.'='.$valueOfID';

ponieważ PHP musi parsować podwójne nawiasy i taki skrypt wykona sie 100% wolniej (czyli 0.0005ms) Najwyraźniej to że string zapisany w ten sposób jest kompletnie nieczytelny i błąd składniowy wynikający z braku spacji czy apostrofu w ważnym miejscu, oraz fakt że samo zapytanie będzie się wykonywać pewnie z 20ms, czyli 40000x dłużej jest pomijalny.
Dlatego ja mam gdzieś mikrooptymalizacje i robię to sposobem do którego "zaraził" mnie Mateusz Świetlicki, choć początkowo miałem opory w stylu "to będzie wolno działało"
   sprintf('select %s from %s where %s=%s', $fields,$table,$id,$valueOfId);

tutaj już mi się cudzysłowy czy spację nie pomylą bo mam oddzielony template stringa od jego zawartości.


Zbliżam się do podsumowania posta. A podsumowanie jest takie: wszystkie optymalizacje wg Google które tutaj naświetliłem w artykule robiłem mając bardzo krótki staż w PHP, myślałem że robię dobrze, a w miarę jak zauważałem do czego to prowadzi, zastępowałem je (lub po prostu nie pisałem tak więcej) solucjami które tu pokazałem. Być może też troche było w tym inspiracji C#. W każdym razie zdenerwowało mnie że ktoś pokazuje błędy beginnerów jako dobre praktyki.

PS. jak patrze na te kawałki kodu z artykułu google, to mam wrażenie że pisali je ci sami programiści którzy pisali moduły do joomli 1.5. Kto się bawił ten będzie wiedział jak wyglądają od środka i dlaczego są takie zabawne :)

środa, 13 czerwca 2012

przeciążąnie GetHashCode

Doszukałem się dziś ciekawego jak dla mnie posta.

metoda T.Equals(T objToCompare) porównuje 2 instancje obiektów, zwracając boolowską wartość, czy są sobie równe czy nie. Przeciąża się to we własnych klasach, aby umożliwić rozmaitym IComparerom porównywanie naszych obiektów. To wiedzą wszyscy.

Metoda ta należy do typu object, więc każda instancja obiektu ma tą metodę.

Nieco większą zagadką jest metoda GetHashCode.
Co prawda visual studio rzuca nam warningiem, jeżeli przeciążymy metodę Equals, nie przeciążąjąc metody GetHashCode. Nie wiem jak inni, ja się w ten warning nie zagłębiałem. Po nazwie metody wywnioskowałem że zwraca ona hash do tablic hashujących czyli np Dictionary. W końcu porównywanie hashy jest zawsze szybsze niż porównywanie całych obiektów, to to nawet na studiach było na 2 roku na AiSD.

A jakoś nie zdarzyło mi się jeszcze używać w Dictionary własnych obiektów które musiałbym porównywać między sobą operatorem Equals() lub ==, więc jakoś sobie radziłem z brakiem tej wiedzy.

Tymczasem dziś przypadkowo znalazłem:

"Dlaczego przeciążąnie GetHashCode() jest ważne? Jeżeli Hash zwrócony przez GetHashCode dla dwóch różnych obiektów nie będzie sobie równy, te obiekty nigdy nie zostaną uznane za równe sobie, i metoda Equals() nie zostanie dla nich uruchomiona nigdy. Powinieneś w pierwszej kolejności przeciążyć GetHashCode() i zwracać ten sam hash dla dwóch równych sobie obiektów".
No proszę.. .NET już domyślnie wykorzystuje ten fajny algorytm o którym się uczyliśmy tak dawno, porównując najpierw hashe obiektów, a dopiero wtedy gdy się zgadzają - uruchamiając funkcję compare (bo przecież może być kolizja hashy i nie można na samych hashach w 100% polegać).

Teraz nawet sobie przypominam że dziwiłem się czego Equals() nie działa w jakims swoim małym programiku.. ale zamiast wgłębiać się w przyczyny, średnio był czas na to przy "nieciekawym" małym programiku, po prostu napisałem porównanie w metodzie "na sztywno" i też działało. I zapomniałem o sprawie.

Dobrze że na to trafiłem, człowiek uczy się całe życie, tym bardziej tak rozbudowanego środowiska jakim jest .NET. Czasem robiąc "ciekawsze" rzeczy, można zapomnieć o podstawach :)


Skoro już jesteśmy przy liczeniu hashy, podzielę się z wami tym co wyniosłem z zabawnego wykładu AiSD który w całości był dyktowany z pożółkłej już ze starości kartki :) (Niech żyje S....! Niech żyje P...!)

Metoda GetHashCode() powinna być raczej prosta i szybka w wykonywaniu niż bardzo dokładna (w sensie rzadko generująca kolizję). Jej celem jest odsianie wstępne większości obiektów, które na pewno nie będą sobie równe z racji róźnych hashy, kolizje i tak zostaną zweryfikowane metodą Equals. Chodzi tu o szybkość porównywania, więc lepiej jeżeli metoda GetHashCode generuje częstsze kolizje (np w 10% przypadków) ale będzie 10x szybsza niż jeżeli wygeneruje kolizje w przypadku 1%, ale będzie 10x wolniejsza. Jeżeli już uruchomi się metoda Equals, w której np. sprawdzimy wszystkie pola klasy metodami Equals, to dla nich też, zanim zostanie uruchomione Equals, zostanie najpierw uruchomione ich GetHashCode, więc ta metoda Equals wcale nie musi być dużo wolniejsza od samego GetHashCode. Może kiedyś wróce do zagadnienia (jak nie będe miał na głowie 7 sprawozdań na jutro), i machnę mały teścik z różnymi strategiami tego, z odpowiedzią która jest najwydajniejsza.

Choć i tak w 90% przypadków wydajność nie będzie miała żadnego znaczenia - a jeśli już bierzemy sie za przetwarzanie dużych zbiorów danych, dlaczego robić to w kodzie a nie użyć do tego np. bazy danych SQL?

Częste kolizje w GetHashCode() mogą jedynie popsuc wydajność w hashowanych kolekcjach (HashMap, HashSet), jeżeli twoja klasa nie bedzie jakoś intensywnie używana w tych kolekcjach, to lepiej zrobić szybszą GetHashCode().

Wg mnie najprostszą metodą na GetHashCode i zapewne wcale nie najgorszą będzie po prostu dodanie do siebie wartości zwróconej przez metodę GetHashCode dla wszystkich pól naszej klasy, i pomnożenie każdej z nich przez inną liczbę pierwszą. To wygeneruje dużo kolizji, ale będzie bardzo szybkie.

Innym sposobem jest hashGenerator wbudowany we framework. Podobno ten generator jest bardzo dobrej jakości (mało kolizji). I nie dublujemy kodu.
return new { A = PropA, B = PropB, C = PropC, D = PropD }.GetHashCode();
Podstawiamy do anonimowego obiektu pola naszej klasy, i uruchamiamy metode GetHashCode. jednak obawiam się że tworzenie za kazdym razem nowego obiektu może nie być wcale takie szybkie jakbyśmy tego oczekiwali. Jednak dla miejsc niekrytycznych dla wydajności kodu, taka implementacja będzie w pełni wystarczająca.

Należy również pamiętać że wiele typów w .NET (również tych wbudowanych) nie gwarantuje że hashe będą stałe pomiędzy kolejnymi uruchomieniami aplikacji. Identyczny obiekt wczytany w 2 różnych instancjach aplikacji prawdopodobnie będzie miał różny hash, więc do porównywania obiektów pomiędzy instancjami trzeba zrobić jakieś dodatkowe ID, na pewno nie powinno się używać do tego hashy z tej metody.

poniedziałek, 11 czerwca 2012

Boje z ASP.NET MVC AJAX i pragma no-cache

Dziś napotkaliśmy w pracy dość ciekawy problem który nie dał się rozwiązać szybko na pierwszy rzut oka.

Pisząc portal w ASP.NET MVC, używając widoków Razor, postanowiłem zdynamizować nieco stronę główną, i zastąpić linki generowane przez helper @Url.Action, linkami pochodzącymi z helpera @Ajax.ActionLink.

Linki te to taka bezpieczna ajaxowa nakładka na nawigację - jest to pełnoprawny, normalny link, który jest indeksowalny dla wyszukiwarek, jednak jeżeli użytkownik ma włączoną obsługę javaScriptu, wtedy kliknięcie w link spowoduje wejście na url linka za pomocą Ajax, i wyświetlenie zawartości w obiekcie o zadanym ID.

Używam tych linków w kluczowych miejscach portalu którym aktualnie się zajmuję - w części w której większość użytkowników spędzi dużo czasu, więc warto zainwestować w ergonomię i pozwolić użytkownikom np. na przeglądanie newsów bez nieco deprymującego odświeżenia strony w przeglądarce.

Ponieważ Ajax.ActionLink prowadzi pod ten sam adres URL, co jego nieajaxowa wersja, ASP.NET posiada metodę obiektu Request dostępnego w każdym kontrolerze i widoku: Request.IsAjaxRequest(). Jeśli ta metoda zwróci true, to oznacza to że widok został użyty w akcji kontrolera która nie została załadowana zwykłym GET-em, tylko właśnie za pomocą ajax.

Dzięki temu, możemy zadecydować, czy widok ma zwrócić całą stronę, wraz z jej parent layoutem, czy tylko samą treść, ponieważ zapytanie jest ładowane przez ajax, i parent layout już istnieje.

przykładowy widok, który może być odczytywany w dwójnasób: zwykłym linkiem GET, oraz ajaxem.


@{
ViewBag.DontDrawMainPagePiece = true;
ViewBag.Title = _LayoutResources.Actuals;
if (Request.IsAjaxRequest() == false)
{
Layout = "~/Views/Shared/Layouts/AjaxLayout.cshtml";
}
else { Layout = null; }
}
@Html.Action( "NewsListPartial", new { id = ViewBag.PageId } )





natomiast AjaxLayout.cshtml zawiera coś takiego


[......]

<div class="content_container" id="ajax_container">
@RenderBody()
</div>
[......]


przykładowy ajax link który działa jako get na stronie bez JS i jako ajax w pełni kompatybilnej przeglądarce, współpracujący z tym wygląda tak:

@{var updateTargetId="ajax_container";}

@Ajax.ActionLink( "link_text", Model.ActionName, Model.ControllerName,
Model.RouteParameters, new AjaxOptions
{
LoadingElementId = "loader_image",
UpdateTargetId = updateTargetId,
OnBegin = String.Format("fadeOut('{0}')",updateTargetId),
OnComplete = String.Format("fadeIn('{0}')", updateTargetId),
HttpMethod = "Post"
} )

taki piękny link, wygeneruje nam coś co odwoła się do akcji z modelu, natomiast wszystko co ten ajax request zwróci, zostanie załadowane do obiektu DOM o id #ajax_container. Dlatego ajax_container jest na zewnatrz, w pliku AjaxLayout.cshtml - jezeli strona jest odczytywana po get - musi zostac zrenderowana z calosci razem z otaczającym treść htmlem. treść znajduje sie w divie o id #ajax_container.

Jesli klikniemy w ajaxlink, to to co zostanie zwrocone zostanie załadowane do diva ajax_container.
Poniewaz jest to request ajaxowy, a w widoku zawarliśmy stwierdzenie że, jeżeli request jest ajaxowy, to layout= null, zostanie zwrócona sama treść, bez kolejnego ajax_container, dzięki czemu przegladarka nie zglupieje. Wszystko sie zgadza, layout pozostal bez zmian, zmieniła się sama treść.


No i tak zrobiliśmy. Po publishu sie okazało że strona rozkraczyła się pod Internet Explorerem 9. Każda przeglądarka interpretowala link poprawnie, i wyświetlała treść poprawnie, natomiast internet Explorer umieszczał w ajax_container, zrenderowaną w całości strone, wraz z otaczającym ją layoutem, strona w stronie, menu 2 razy, efekt opłakany. Pod każą inną przeglądarką wszystko działało poprawnie. So confusing... błąd jest ewidentnie po stronie serwera więc jak może go powodować inna przeglądarka?

Sprawdziliśmy snifferem Fiddler co się dzieje na łączach. Okazało się że z jakiegoś powodu prawie wszystkie akcje w responsach zwracają pragma: no-cache, jednak strona główna z jakiegoś powodu nie. I ta drobna różnica stała się przyczyną błędu i zamieszania.

Uruchomiłem debugger, rozstawiłem breakpointy. Internet explorer wykonywał request na Home/Index, ten go przekierowywał do właściwej strony, jednak, on miał już u siebie w cache skeszowaną wersję tej strony, pobraną przy pierwszym wejściu na stronę główną (za pomocą GET), wraz z całym layoutem (choć tym razem request był ajaxowy, i pomimo że ASP.NET, już się wziął za ponowne zrenderowanie tego widoku (tym razem bez tej całej otoczki zawartej w ajaxLayout.cshtml), Internet explorer zwrócił stronę z cache, wraz z menu, stopką, nagłówkiem...).

Natomiast np google chrome czy mozilla, ponieważ w piewszym response nie było wyraznego polecenia cache-owania danego URL-a, nie robiły tego, i strona ładowała się jak należy.

Problem wynika z tego że IE oraz reszta różnie interpretują parametr w nagłówku HTTP o nazwie pragma.
Internet explorer, gdy go nie znajdzie, to zakłada że strone trzeba scache-ować.
Natomiast reszta przegladarek zakłada że gdyby developer chcial by strona była cache-owana, to by umiescił tam pragma-cache i jakąś wartość.

powyższy kawałek kodu w global.asax, w event handlerze PreRequestExecuteHandler załatwia sprawę:

this.Response.AddHeader( "Cache-Control", "no-cache, no-store, max-age=0, must-revalidate" );

od tej pory wszystkie strony funkcjonują jednakowo we wszystkich przeglądarkach pod względem cache.