Skip to content
This repository

Memory Issue with Sortable DataObject Decorator #11

Open
gordonbanderson opened this Issue June 20, 2011 · 3 comments

2 participants

Gordon Anderson Saskoh42
Gordon Anderson

I've been tracing a memory leak for some time and have finally tracked it down to the onBeforeWrite method of the SortableDataObject decorate. The symptom I saw was not being able to save a single Image due to memory constraints, and the memory jumping from 29M to > 128M when saving the Image.

The culprit is here

public function onBeforeWrite()
        {
        if(!$this->owner->ID) {
            if($peers = DataObject::get($this->owner->class))
                $this->owner->SortOrder = $peers->Count()+1;
        }
    }   
 } 

The line

  if($peers = DataObject::get($this->owner->class))

is loading all of the DataObjects of a particular class, e.g. Image, into memory, counting them, and setting the SortOrder to be one more than the maximum.

As a site gets bigger this will result in more memory being used when saving an object of a given class, eventually resulting in a memory leak preventing objects being saved.

In short, the number of objects of a given DataObject class are effectively limited by the amount of RAM allocated to the PHP process.

Saskoh42

Hello,

this issue occured today for me, too. I solved it by changing the method "onBeforeWrite" in "SortableDataObject" to the following:

if(!$this->owner->ID) {
    $sortOrder = 0;
    $records   = DB::query(
        sprintf(
            "SELECT
                MAX(SortOrder) AS maxSortOrder
            FROM
                File
            WHERE
                ClassName = '%s'",
            $this->owner->class
        )
    );

    if ($records) {
        foreach ($records as $record) {
            $sortOrder = $record['maxSortOrder'];
            break;
        }

        $sortOrder = (int) $sortOrder + 1;
    }
    $this->owner->SortOrder = $sortOrder;
}

This solves the memory consumption issue.

@unclecheese: could this be patched into DOM for future versions?

All the best,
Sascha

Gordon Anderson

Saksoth's code can be found here gordonbanderson@3beba35

Gordon Anderson

I think this has been resolved here, JonasAleknavicius@aab754f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.