1915's avatar
Level 1

save() inside a loop leaks memory

Surely I'm doing something wrong, but creating new records inside a loop takes up more and more memory for each loop, until it runs out of memory all together and breaks.

I first figured I could fix it with just creating an array and do a bulk insert with DB::table()->insert() but then I realised that I need the id for every created record.

                foreach($files as $f) {
                    unset($contents);
                    echo $f." : ".memory_get_usage()."\n";
                    $contents = file_get_contents($f);
                    unset($new_document);
                    $new_document = new \App\document();
                    $new_document->email_id = $document->email_id;
                    $new_document->directory = 'in';
                    $new_document->filename = $document->classified_as.time().'.pdf';
                    $new_document->ocr_task_status = 'Unknown';
                    $new_document->key = str_random(32);
                    $new_document->authority_id = $document->authority_id;
                    $new_document->pages = 1;
                    $new_document->step = 'email';
                    $new_document->classified_as = $document->classified_as;
                    $new_document->classified_score = $document->classified_score;
                    $new_document->identified = Carbon::now()->toDateTimeString();
                    $new_document->classified = Carbon::now()->toDateTimeString();
                    $new_document->save();

                    $res = Storage::disk('s3')->put('in/' . $new_document->id, $contents);
                    unlink($f);
                }
0 likes
13 replies
StuffedGoat's avatar

Allow me to post as a novice.

Would it not be less memory consuming to put the store-method outside of the foreach like so:

foreach($files as $f) {
    // ...
}
$res = Storage::disk('s3')->put('in/' . $new_document->id, $contents);
1915's avatar
Level 1

Thank you for replying.

I removed the Storage line for testing and it had no effect on the memory problem.

pmall's avatar

What if you put the unset at the end of the loop (after storing the file).

1915's avatar
Level 1

Unfortunately, that did not work either.

pmall's avatar

It is hard to see where this go wrong with just this piece of code. Are you really sure this comes from the creation of document eloquent model ?

phildawson's avatar

I would always advise starting with the most basic and then test and keep adding until you see the cause.

foreach($files as $f) {
    echo $f." : ".memory_get_usage()."\n";
    $contents = file_get_contents($f);
}

then if fine try

foreach($files as $f) {
    echo $f." : ".memory_get_usage()."\n";
    $contents = file_get_contents($f);
    $new_document = new \App\document();
}

and so on..

You can also slime the expected values above the loop to test without a real $document.

$document = new StdClass;
$document->classified_score = 34;
$document->authority_id = 4;
etc
1915's avatar
Level 1

Actually removing the Storage::disk line again came up with no memory leak, which is totally different from when I tried it yesterday. Must have forgotten to save the file when before I ran the test... However, running this piece of code works, unexpectedly:

                $files = glob(base_path().'/storage/app/'.$document->id.'-*.pdf');
                foreach($files as $f) {
                    echo $f." : ".memory_get_usage()."\n";
                    $contents = file_get_contents($f);
                    Storage::disk('s3')->put('in/delete_me', $contents);
                    unset($contents);
                }

Getting down to it, this causes a memory leak:

                    Storage::disk('s3')->put('in/' . $new_document->id, $contents);

And this doesn't:

                    Storage::disk('s3')->put('in/' . 34, $contents);

So why oh why is referencing $new_document->id causing a memory leak?

luceos's avatar

Could you try the following:

foreach($files as $f) {
                    echo $f." : ".memory_get_usage()."\n";
                    $contents = file_get_contents($f);
                    $new_document = new \App\document();
                    $new_document->email_id = $document->email_id;
                    $new_document->directory = 'in';
                    $new_document->filename = $document->classified_as.time().'.pdf';
                    $new_document->ocr_task_status = 'Unknown';
                    $new_document->key = str_random(32);
                    $new_document->authority_id = $document->authority_id;
                    $new_document->pages = 1;
                    $new_document->step = 'email';
                    $new_document->classified_as = $document->classified_as;
                    $new_document->classified_score = $document->classified_score;
                    $new_document->identified = Carbon::now()->toDateTimeString();
                    $new_document->classified = Carbon::now()->toDateTimeString();
                    $new_document->save();

                    $res = Storage::disk('s3')->put('in/' . $new_document->id, $contents);
                    unlink($f);
                    unset($res, $new_document, $contents);
                }

This will clean up as much memory as possible within the single iteration.

Also it might be possible that saving the document causes the memory usage. You could therefor try to run a raw insert query instead, preventing the generation of an object.

1915's avatar
Level 1

That doesn't work either.

1915's avatar
Level 1

By placing

$new_document = new \App\document();

outside of the foreach loop, the memory leak is dealt with. However this just updates the same record over and over again in the database, since neither $new_document->__construct() nor $new_document->newInstance() creates a new record.

But at least I'm getting closer...

zenichanin's avatar

Hey @1915, did you ever figure this one out? I'm also facing the same memory leak issue when running new Model and save() method in a large loop. Unsetting the model doesn't seem to do anything.

EDIT: Actually mine was caused by Debug bar. I guess it was logging all queries and thus increasing memory considerably each time. Once I disabled Debug bar, it worked pretty flawlessly.

1 like
ykchan's avatar

@zenichanin Thank you so much! In my case, it is Telescope that caused the issue. Disabling Telescope recording solved it

baubomas's avatar

If anyone finds this isssue, using fopen instead of file_get_contents fixed the problem for me.

$contents = fopen($f, 'r+');
$res = Storage::disk('s3')->put('in/' . $new_document->id, $contents);
fclose($contents);

Please or to participate in this conversation.