2013-08-15 22 views
6

Mam dość duży program, który generalnie działa cudownie, ale wykorzystuje szalone ilości pamięci do uruchomienia. Jest to podejście do uczenia maszynowego, które gromadzi dużo danych, a więc ogólnie jest w porządku, ale pamięć rośnie i rośnie bardzo szybko, nawet po zebraniu wszystkich danych, więc użyłem masywu valgrind, aby dowiedzieć się, co jest nie tak. Wierzchołek drzewa sterty masywu wygląda tak:Moja pamięć nie jest wolna

99.52% (60,066,179B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc. 
->43.50% (26,256,000B) 0x439785: Image::Image(SDL_Surface*) (Image.cpp:95) 
| ->43.50% (26,256,000B) 0x437277: EncodedFeature::forwardPass() (EncodedFeature.cpp:65) 
.... 

Pomyślałem więc, hmm, może skonstruowane Obraz nie jest free'd, ale nie:

void EncodedFeature::forwardPass() 
{ 
    // Get image: 
    Image* img = new Image(screen); 

    // Preprocess: 
    if(preprocessor) 
     preprocessor->process(img); 

    // Do forward pass: 
    encoder->encode(img, features); 

    delete img; 
} 

Więc do konstruktora Obrazek:

Image::Image(SDL_Surface* surface) 
{ 
    this->width = surface->w; 
    this->height = surface->h; 
    pixels = new int*[width]; 
    for(int i = 0; i < width; i++) 
     pixels[i] = new int[height]; 

    for(int x = 0; x < surface->w; x++) 
     for(int y = 0; y < surface->h; y++) 
      pixels[x][y] = getPixelFromSDLSurface(surface, x, y); 
} 

Wystarczy alokuje tablicę pikseli, która zostanie zwolniona w destructor później:

Image::~Image() 
{ 
    if(!pixels) 
     return; 

    for(int x = 0 ; x < width; x++) 
     delete[] pixels[x]; 

    delete[] pixels; 
} 

Więc ostatni winowajcą:

Uint32 Image::getPixelFromSDLSurface(SDL_Surface *surface, int x, int y) 
{ 
    if(!surface) 
     return 0; 

    // Got this method from http://www.libsdl.org/cgi/docwiki.fcg/Pixel_Access 
    int bpp = surface->format->BytesPerPixel; 
    /* Here p is the address to the pixel we want to retrieve */ 
    Uint8 *p = (Uint8 *)surface->pixels + y * surface->pitch + x * bpp; 

    switch(bpp) { 
    case 1: 
     return *p; 
     break; 

    case 2: 
     return *(Uint16 *)p; 
     break; 

    case 3: 
     if(SDL_BYTEORDER == SDL_BIG_ENDIAN) 
      return p[0] << 16 | p[1] << 8 | p[2]; 
     else 
      return p[0] | p[1] << 8 | p[2] << 16; 
     break; 

    case 4: 
     return *(Uint32 *)p; 
     break; 

    default: 
     return 0;  /* shouldn't happen, but avoids warnings */ 
    } 
} 

Jak wspomniano w komentarzu, że mam jeden z SDL wiki, więc mam nadzieję, że to nic nie przecieka. bpp w moim przypadku jest tak naprawdę zawsze 1, więc po prostu zwraca int pod adresem p, co nie brzmi dla mnie nieszczelnie.

Jestem na końcu mojego rozumu tutaj. Czy ktoś może pomyśleć o tym, gdzie poszło wspomnienie? Mam na myśli, że masyw wskazuje konkretnie na konstruktor obrazu, ale nie widzę niczego złego ...

Wielkie dzięki za rozpatrzenie mojego problemu!

Max


odpowiadając na komentarze:

Masz rację, że nie potrzeba img jako wskaźnik. Pochodzę z tła Java, więc chcę, żeby wszystko było wskazówkami :) Zmieniłem to, ale nie pomogło.

Linia 95 jest wewnątrz pierwszej pętli konstruktora: pixels[i] = new int[height];

W jednym z preprocesorów robię zmienić rozmiar obrazu, ale mam to nazywając moją funkcję resetowania, który powinien upewnić się, że stara tablica zostanie usunięty:

void Image::reset(int width, int height) 
{ 
    if(pixels) 
    { 
     // Delete old image: 
     for(int x = 0 ; x < width; x++) 
      delete[] pixels[x]; 

     delete[] pixels; 
    } 

    this->width = width; 
    this->height = height; 
    pixels = new int*[width]; 
    for(int i = 0; i < width; i++) 
     pixels[i] = new int[height]; 
} 

Po czym ja napełnić wartości pikseli ...

żadne wyjątki są wyrzucane gdziekolwiek.

Gdzie mógłbyś zaproponować używanie inteligentnych wskaźników?

Dzięki za wszystkie odpowiedzi!

+6

Proponuję użyć wektorów/inteligentnych wskaźników/odpowiedniej alternatywy zamiast 'new' i' delete'. – chris

+1

Czy coś rzuca wyjątki i brakuje tego usunięcia? – doctorlove

+1

Która linia ma numer 95 w pliku 'Image.cpp'? –

Odpowiedz

6

Nie sądzę, że prawidłowo reprezentujesz piksele w klasie obrazu. Myślę, że możesz użyć prostego 1-wymiarowego vector prawidłowego typu bez znaku (uint32_t?).

(w nagłówku):

class Image { 
protected: 
    std::vector<uint32_t> pixels; 
    ... 
}; 

(w pliku wdrażania)

size_t Image::offset(unsigned x, unsigned y) { 
    return (y * width) + x; 
} 

Image::Image(const SDL_Surface* surface) 
{ 
    width = surface->w; 
    height = surface->h; 
    pixels.reserve(width * height); 
    for(unsigned x = 0; x < width; x++) 
     for(unsigned y = 0; y < height; y++) 
      pixels[offset(x, y)] = getPixelFromSDLSurface(surface, x, y); 
} 

Twój destruktora będzie wówczas całkowicie pusty, bo nie ma nic za to zrobić:

Image::~Image() { 
} 

Uwaga: musisz użyć metody offset(), aby uzyskać prawidłowy indeks do vector dla dowolnej pary x/y.

+0

Wielkie dzięki za to, nigdy byś o tym nie pomyślał! Po prostu próbowałem i wydaje się, że działa. Prowadzę teraz eksperyment i patrzę na użycie pamięci. Tylko błąd, który znalazłem: W offsecie powinno to być 'return (y * width) + x;' (odwrotnie). Dzięki za banda! – cpury

+0

@cpury Tak, powinno :) Będę aktualizować odpowiedź. – trojanfoe

1

Najbardziej prawdopodobnym scenariuszem jest to, że twój reset lub destruktor nie jest wywoływany w jakiejś ścieżce kodu, powodując wyciek tej pamięci.

Na szczęście C++ ma wbudowaną funkcję zapobiegania takim wyciekom: nazywa się vector.

Więc najpierw dokonać pixels wyglądać następująco:

typedef std::vector<int> PixelCol; 
typedef std::vector<PixelCol> Pixels; 
Pixels pixels; 

Teraz rozmiar w konstruktorze:

Image::Image(SDL_Surface* surface) 
: pixels(surface->w, PixelCol(surface->h)) 
{ 
    this->width = surface->w; 
    this->height = surface->h; 

    for(int x = 0; x < surface->w; x++) 
     for(int y = 0; y < surface->h; y++) 
      pixels[x][y] = getPixelFromSDLSurface(surface, x, y); 
} 

koniec aktualizujemy reset:

void Image::reset(int width, int height) 
{ 
    Pixels(width, PixelCol(height)).swap(pixels); 
} 

Pozwalając język zarządza pamięcią, całkowicie eliminujesz taką pamięć błąd zarządzania grzechu twój kod.

+0

Wygląda podobnie do odpowiedzi @trojanfoe, więc jestem pewien, że rozwiąże to również mój problem! Dzięki! – cpury

Powiązane problemy