Skip to content

Memory Issue with Sortable DataObject Decorator #11

Open

Description

@gordonbanderson

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.

Activity

  1. Saskoh42 commented on Jan 27, 2012

    @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

  2. gordonbanderson commented on Jun 2, 2012

    @gordonbanderson
    Author

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

  3. gordonbanderson commented on Nov 16, 2012

    @gordonbanderson
    Author

    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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions