Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory Issue with Sortable DataObject Decorator #11

Open
gordonbanderson opened this issue Jun 20, 2011 · 3 comments
Open

Memory Issue with Sortable DataObject Decorator #11

gordonbanderson opened this issue Jun 20, 2011 · 3 comments

Comments

@gordonbanderson
Copy link

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
Copy link

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

@gordonbanderson
Copy link
Author

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

@gordonbanderson
Copy link
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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants