Use XMLWriter class instead of fwrite()

The D6 version needs to add a requirements check to ensure the class exists, as XMLWriter is included by default only as of PHP 5.1.2.

Initial performance testing:

w/o patch - small set

Requests per second:    2.02 [#/sec] (mean)
Time per request:       495.363 [ms] (mean)
Time per request:       495.363 [ms] (mean, across all concurrent requests)
Transfer rate:          0.81 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:   419  495  65.5    479     834
Waiting:      419  495  65.5    479     834
Total:        419  495  65.5    479     834


w/ patch - small set

Requests per second:    2.01 [#/sec] (mean)
Time per request:       498.409 [ms] (mean)
Time per request:       498.409 [ms] (mean, across all concurrent requests)
Transfer rate:          0.80 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   1.4      0      20
Processing:   427  498  66.9    482     831
Waiting:      427  498  66.9    482     831
Total:        427  498  66.9    484     831

--------------------------------------------------------------------------------

w/o patch - medium set

Requests per second:    0.26 [#/sec] (mean)
Time per request:       3777.781 [ms] (mean)
Time per request:       3777.781 [ms] (mean, across all concurrent requests)
Transfer rate:          0.11 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:  3685 3778  82.3   3760    4065
Waiting:     3685 3778  82.3   3760    4065
Total:       3685 3778  82.3   3760    4065


w/ patch - medium set

Requests per second:    0.26 [#/sec] (mean)
Time per request:       3806.549 [ms] (mean)
Time per request:       3806.549 [ms] (mean, across all concurrent requests)
Transfer rate:          0.11 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:  3695 3806  90.4   3788    4167
Waiting:     3695 3806  90.4   3788    4167
Total:       3695 3806  90.4   3788    4167

So we get a marginal time difference. However, the memory usage shows something interesting:

At end of generation, the result of memory_get_peak_usage(TRUE):
Without patch: 7077888
With patch: 6815744

Patch saved about 256 KB with about 7000 nodes. I'm not sure if this should proceed or not.

Yeah I think I need to test with an even bigger data set. The percentage difference for memory usage was way more than the percentage difference in execution.

I created a 'test.php' file in my Drupal's root folder:

include_once './includes/bootstrap.inc';
drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL);
module_load_include('generate.inc', 'xmlsitemap');
xmlsitemap_run_progressive_batch('xmlsitemap_regenerate_batch');
exit();

And then run the apache ab benchmarking command & record results from before & after patch:
ab -c 1 -n 25 http://mysql.drupal7dev.local/test.php

If you have a smaller data set, you can change the 'n' paramter to a higher value, but for a large data set I wouldn't recommend going over 25 loops.

Ok so I think I've finally come up with a compelling reason why we should make this switch. #786510: Contexts should be able to alter the XML header output (and possibly link output?) needs other modules to be able to modify the XML schema and individual XML link elements. That's super easy to do with XMLWriter were we can pass the writer object or the element object to the alter functions. Currently it's not easy to do otherwise.

Here's the patch against the current DRUPAL-6--1 branch.

Anyone? :/

I'm not sure how I'm going to handle that. I may just remove the $status handling everywhere. At this point I'm more concerned about performance testing to see that this isn't going to be horrible change.

Yeah the problem is that we need to allow other modules to alter stuff in the XML head, namespaces, etc. I would think we'd get better performance using the native XMLWriter rather than generating large amounts of arrays (or a large number of separate small arrays). I'm going to move ahead with this issue this weekend as it's blocking other good patches from going in. If we encounter any major problems (none discovered in testing so far), it is possible to rollback.

I'm going to proceed with this since it has not gotten any reviews and is blocking a major feature.

OMG yes this is exactly the approach I want to do because this allows us to extend the XMLWriter into our own XMLSitemapWriter class and also allows us to override XMLWriter::flush to keep track of the file writing status.

Revised patch

Committed to CVS!
http://drupal.org/cvs?commit=413418

Committed this to CVS on 6.x-2.x as well.
http://drupal.org/cvs?commit=413422