Moodle

We have resolved the problem

• commenting out the call to backup_flush() of every StartElement... and
• including the code marked with + in the file restorelib.php at the end of the function restore_read_xml(...)

+ $lasttime = time(); // crmas
while ($data = fread($fp, 4096) and !$moodle_parser->finished) {
+ if ((time() - $lasttime) > 5) { + $lasttime = time(); + backup_flush(1); + }
xml_parse($xml_parser, $data, feof($fp))
or die(sprintf("XML error: %s at line %d",
xml_error_string(xml_get_error_code($xml_parser)),
xml_get_current_line_number($xml_parser)));
}
fclose($fp);

This modification has achieved a reduction in the execution time from >6000 seconds to 600 secs (moodle.xml was larger than 700.000 lines)

Hi Carlos,

thanks for your patch. I'm taking a look to it right now...stay tuned... ciao

Adding Penny here to hear her opinion.

I've found this could be a nice path to follow:

1) Deprecate the second parameter of backup_flush($n=0,$time=false). I haven't found any usage and we could notice it for now and take it out for 2.0.

2) I would leave all the current calls to backup_flush() in all the restore_create_xxxx(), restore_user_() because they can need such flush enabled there.

3) Will delete all the backup_flush() from all the startElementXXX() handers as suggested by Carlos.

4) Will add them to the main fread as suggested by carlos (controlling the SILENTY flag, of course)

With this we avoid millions of calls to the backup_flush() function, adding content by time basis). How does this sound for you Penny? And one more question... why the backup_flush() function itself is using another different RESTORE_SILENTLY_NOFLUSH define? I think I asked this before but I don't remember the answer... (blush)

Ciao

Sounds good to me!

The reason for the different constant was... (I was trying to remember this the other day and I don't actually remember, I'm guessing) that there's a difference between printing stuff to STDOUT and flushing the output buffer. The two different cases are:

wanting to call backup functions without actually having <li>doing blah blah</li> printed to the screen (eg one click backup), and avoiding browser timeouts. iirc the flushing is just to prevent browser timeouts?

There are three different times backup happens:

• interactively (want to flush buffer AND print the <li> stuff) still in browser
• non interactively in browser (want to flush buffer but not print <li>)
• non interactively in CLI (cron, scripts), don't want flush buffer or <li>s

I've reassigned it to Yu to see if it's safe finally. Sorry for the delay, 100% my fault (just re-discovered this some minutes ago).

Hi, I have tested this using the feature's demo backup in MDL-9602.

Before patch time taken to execute restore is 11 seconds
After patch time taken to execute restore is 7 seconds

I am not seeing a significant time improvement probably because this course isn't really that big. (BIG backup would help!)

Restored course seems normal after patch.

I don't mind putting this into 1.8, 1.9 and HEAD, waiting for Eloy's go ahead =)

Cheers,

Yu

Well done Yu! B-)

Can people, please, test BIG restores under 1.8, 1.9 and 2.0 to see if they experiment nice speed-ups?

TIA and ciao

Tried to make an artificially big xml file, although wasn't that big, but there are definite improvement with the patch:

before:
26.933392 secs
after:
22.739394 secs

Closing as fixed. Ciao

I ran into a problem around the same lines of code related to this issue: Restore failing because xml_parse() crashes.

I don't think this issue is the cause of my problem but I was wondering if any of you guys could take a look at it:

http://tracker.moodle.org/browse/MDL-18825

I'm not seasoned enough in PHP to be able to figure this one out. Any help would be greatly appreciated. Thank you!

This was fixed a long time ago.