XML source plugin does not scale

Ah! Found a couple of examples on the web, once we've got an element representing a source object, we can feed it into DOM/SimpleXML and use xpath as we normally do to pull values from it. So, it's really only the $item_xpath argument to the MigrateItemsXML constructor where we don't have built-in xpath capability and need to come up with a custom solution.

Exactly - at this moment (which might change in another 10 minutes;) I'm looking at doing

class MigrateXMLReader extends XMLReader implements Iterator {
  public function __construct($xml_url, $item_xpath='item') {
  } 
}

Iterating over each match to the $item_xpath. The MigrateItems implementation would encapsulate this. Or, the MigrateItems implementation would implement Iterator and encapsulate an XMLReader instance...

For your review... I did implement MigrateXMLReader much as suggested above, wrapped inside a new MigrateSource named MigrateSourceXML. You pass a URL, element query, and ID query much like the existing MigrateItemsXML, but the element query is more restrictive - it does not support the full xpath language, you need to give an explicit path to the element(s) you want (i.e., '/producers/producer' rather than '//producers'). You may also filter on attribute value (e.g., '[@attribute="value"]'). Another feature I found a need for at the last minute is that you can derive your own class from MigrateXMLReader and substitute it.

@drewish: As for the time to count a big document, that's a big enough problem for my current project that I plan on implementing a skip_count feature on sources. Issue to be opened momentarily...

It helps to attach the patch...

#1341776: Option to skip counting

Well, let me give that a try... I have my 353MB XML file, containing something like 70K elements to be turned into nodes and 1M comments, and a migration for 1291 elements going into a video content type, about half skipped by prepareRow(). I added instrumentation around the DOM handling stuff to try

$ drush mi video --instrument=timer
Processed 1291 (617 created, 0 updated, 0 failed, 674 ignored) in 82.6 sec (448/min) - done with 'Video'
...
 MigrateXMLReader::next                             35.493     1292   27.471 
...
 MigrateXMLReader::next inner                       1.429      1291   1.107 
...

I'm thinking there's not a lot of optimization to be done there, but I'll give it a try to give it a shot tomorrow.

Replacing the DOM stuff with

  $this->currentElement = simplexml_load_string($this->readOuterXML());

and running

drush mr video; drush mi video --instrument=timer

three times, the inner section takes on average 8.0% of the MigrateXMLReader::next time (a little over 2ms per call). The DOM method averages 4.2%, just a bit over 1ms, so despite how the code may look, it really is faster.

Committed.

Oops. Upgrading to PHP 5.3 generates strict warnings:

Strict warning: Declaration of MigrateXMLReader::next() should be compatible with that of XMLReader::next() in require_once() (line 582 of /Users/mryan/Sites/m7/sites/all/modules/migrate/plugins/sources/xml.inc).

Problems - we're extending XMLReader and implementing Iterator, each of which declares their own next() method. As it is, things work for our purposes - we've implemented next() for Iterator, which is the only next() we really need - but it's incorrect, and if someone were to try using a MigrateXMLReader as an XMLReader they would find themselves in trouble calling next().

So, what I need to do is encapsulate an XMLReader object in MigrateXMLReader, rather than extend the class.

All right, MigrateXMLReader is no longer an XMLReader itself, it contains member $reader.