2013-08-15 12 views
6

Ho un programma piuttosto grande che generalmente funziona in modo meraviglioso, ma utilizza quantità folle di memoria per l'esecuzione. Questo è un approccio di apprendimento automatico che raccoglie molti dati, e quindi in generale va bene, ma la memoria cresce e cresce molto velocemente anche dopo che tutti i dati sono stati raccolti, e così ho usato valgrind massiccio per scoprire cosa non va. La cima dell'albero mucchio di massiccio si presenta così:La mia memoria non è libera

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) 
.... 

Così ho pensato, hmm, forse l'immagine costruita non è free'd, ma no:

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; 
} 

Quindi, per il costruttore Image:

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); 
} 

Appena alloca una matrice di pixel che viene liberato nel distruttore seguito:

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

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

    delete[] pixels; 
} 

Quindi l'ultima colpevole:

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 */ 
    } 
} 

Come accennato nel commento, ho ottenuto che uno dal wiki SDL, quindi spero che non v'è nulla che perde lì. bpp nel mio caso è in realtà sempre 1, quindi restituisce solo l'indirizzo p, che non suona per me.

Sono qui alla fine del mio ingegno. Qualcuno può pensare a dove è andata la memoria? Voglio dire, massifichiamo punti specifici per il costruttore Image, ma non riesco a vedere nulla di sbagliato lì ...

Grazie mille per aver guardato il mio problema!

Max


rispondere alle vostre osservazioni:

Hai ragione, non ho bisogno di IMG come un puntatore. Vengo da uno sfondo Java quindi voglio solo che tutto sia puntatore :) Modificato ma non aiutato.

Linea 95 è all'interno del primo ciclo for del costruttore: pixels[i] = new int[height];

In uno dei preprocessori ho faccio ridimensionare l'immagine, ma lo faccio chiamare la mia funzione di reset, che dovrebbe assicurarsi che il vecchia serie viene cancellata:

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]; 
} 

Dopo che io riempio i valori dei pixel ...

Nessuna eccezione vengono gettati ovunque.

Dove suggeriresti di utilizzare puntatori intelligenti?

Grazie per tutte le risposte!

+6

Suggerisco di usare vettori/puntatori intelligenti/alternative appropriate invece di 'new' e' delete'. – chris

+1

C'è qualcosa che lancia eccezioni e manca l'eliminazione? – doctorlove

+1

Quale riga è il numero 95 nel file 'Image.cpp'? –

risposta

6

Non penso che tu stia rappresentando i pixel correttamente nella tua classe immagine. Penso che tu possa utilizzare un semplice vector unidimensionale del tipo non firmato corretto (uint32_t?).

(nell'intestazione):

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

(nel file di implementazione)

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); 
} 

tuo distruttore sarebbe poi essere completamente vuota, in quanto non v'è nulla per consentirle di operare:

Image::~Image() { 
} 

Nota è necessario utilizzare il metodo offset() per ottenere l'indice corretto nello vector per ogni coppia data/ora.

+0

Grazie mille per tutto questo, non avrei mai pensato a quello da solo! L'ho appena provato e sembra funzionare. Esecuzione di un esperimento ora e guardando l'utilizzo della memoria. Solo errore Ho trovato: In offset, dovrebbe essere 'return (y * width) + x;' (viceversa). Grazie mille! – cpury

+0

@cpury Sì, dovrebbe :) Aggiornerò la risposta. – trojanfoe

1

Lo scenario più probabile è che in qualche modo il ripristino o il distruttore non venga chiamato in qualche percorso di codice, perdendo quella memoria.

Fortunatamente C++ ha una funzionalità integrata per prevenire tali perdite: Si chiama vector.

Quindi, prima si effettua la pixels simile a questa:

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

Ora abbiamo dimensioni nel costruttore:

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); 
} 

Infine Aggiorniamo reset:

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

Lasciando la lingua gestisce la tua memoria, elimina completamente qualsiasi tipo di memoria bug di gestione peccato il tuo codice.

+0

Sembra simile alla risposta di @trojanfoe, quindi sono sicuro che risolverebbe anche il mio problema! Grazie! – cpury

Problemi correlati