CRM-20754 - Clear memory leak in CSV CLI import #10537
Conversation
MegaphoneJon
commented
Jun 21, 2017
•
- CRM-20754: memory leak in CLI CSV import
|
@PalanteJon can you fix the style error https://test.civicrm.org/job/CiviCRM-Core-PR/15884/checkstyleResult/new/ |
bin/cli.class.php
Outdated
| @@ -430,6 +430,14 @@ public function run() { | |||
| continue; | |||
| } | |||
| $this->row++; | |||
| if ($this->row % 1000 == 0) { | |||
| // Reset PEAR_DB_DATAOBJECT cache to prevent memory leak | |||
| $GLOBALS['_DB_DATAOBJECT']['RESULTS'] = array(); | |||
There was a problem hiding this comment.
Are there any side effects of clearing the results cache? If not then would it be an option to disable it completely? Presumably this would also resolve the large import problem that this ticket addresses.
There was a problem hiding this comment.
@mickadoo No side effects in this scenario, no. However, I can't guarantee there would be no side effects Civi-wide, so I don't think disabling the cache is a worthwhile endeavor.
This object is the one that CRM_Core_DAO is extending, so it's very low-level. While there DOES appear to be an inherited class called DB_DataObject_Overload that we could extend instead, I think we'd find it rather difficult to only disable the cache when it was relevant.
@totten @eileenmcnaughton If you're more familiar with the DB_DataObject->_resultFields property than I am, please feel free to comment.
There was a problem hiding this comment.
Don't we normally use CRM_Core_DAO::freeResult() - which erm mostly works
There was a problem hiding this comment.
Thank you Eileen! CRM_Core_DAO::freeResult() looks slower but functionally equivalent - that is, there's still no "disable the $_resultFields" option. However, that method might result in lower memory use. I have some massive datasets at the moment, so next week when I'm not mid-import, I will do speed and RAM usage tests to compare.
|
I ran some tests tonight and confirmed that |
|
Looks good to me makes sense |
|
Yup works fine. Merging it now |