Consider removing gc_collect_cycles() from DataObject::destroy()

[kinglozzer]

I have a GridField with around 800 items in it, removing gc_collect_cycles() halves the CPU time for print or CSV exports (both GF components call $item->destroy() on each DataObject) and has no impact whatsoever on memory usage, or peak memory usage.

Is PHP clever enough nowadays that we can just leave it alone to handle this itself?

[chillu]

How does that affect mem usage? Removing from milestone, since it's marked as low impact

[kinglozzer]

XHProf runs suggest there’s absolutely no impact on memory at all. I’ve run two types of test:

• benchmarking the request for a GridField print view (~800 items)
• creating, and then subsequently deleting, DataObjects and triggering destroy():

for ($i = 0; $i < 1000; $i++) {
    TestObject::create()->update(['Foo' => 'Bar'])->write();
}

foreach (TestObject::get() as $item) {
    $item->delete();
    $item->destroy();
}

die('done');

gc_collect_cycles() doesn’t cause such a (relatively) large slowdown in the second example, because most of the time is taken up with database queries - it adds around 10% extra CPU time for 1000 objects created/deleted.

I noticed this when benchmarking why the GridField print and CSV export buttons were so slow for my app. There’s only one database query to fetch all the items for rendering, but then ->destroy() is called on each of them so the relative time that gc_collect_cycles() is taking is much higher, around 50%.

So we could either:

• Remove gc_collect_cycles() from DataObject::destroy()
• Remove calls to DataObject::destroy() from GridFieldPrintButton::generatePrintData() and GridFieldExportButton::generateExportFileData()

[dhensby]

what version of PHP are you using? I have no doubt PHP 7 is much better at this now.

I'd be happy to get it out of SS 4, maybe SS 3 depending on impact on older versions of PHP...

[kinglozzer]

Tested on 5.6, I don't have XHProf set up for 7 (I don't know if there's even a PHP 7 compatible version) On Fri, 1 Sep 2017 at 18:05, Daniel Hensby notifications@github.com wrote:

what version of PHP are you using? I have no doubt PHP 7 is much better at this now.

I'd be happy to get it out of SS 4, maybe SS 3 depending on impact on older versions of PHP...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/silverstripe/silverstripe-framework/issues/7326#issuecomment-326633863, or mute the thread https://github.com/notifications/unsubscribe-auth/ABlC_D4aq42vheW32_DWJrRag8Nskt4sks5seDlQgaJpZM4PHbpQ .

[chillu]

Sorry I just realised you've addressed memory usage in your original report - too much skimming during bug triage :) In that case, happy to review a PR removing gc_collect_cycles() from SS4, it's always been hacky.

[tractorcow]

PR https://github.com/silverstripe/silverstripe-framework/pull/7334