DataObjectManager icon indicating copy to clipboard operation
DataObjectManager copied to clipboard

Memory Issue with Sortable DataObject Decorator

Open gordonbanderson opened this issue 13 years ago • 3 comments

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.

gordonbanderson avatar Jun 20 '11 08:06 gordonbanderson

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

Saskoh42 avatar Jan 27 '12 13:01 Saskoh42

Saksoth's code can be found here https://github.com/gordonbanderson/DataObjectManager/commit/3beba35dd20e04b185187007d1aff2672b0bf6b4

gordonbanderson avatar Jun 02 '12 07:06 gordonbanderson

I think this has been resolved here, https://github.com/JonasAleknavicius/DataObjectManager/commit/aab754f8d9d1d9a92922c2b23928ea7530368cea

gordonbanderson avatar Nov 16 '12 11:11 gordonbanderson