XML::Simple is known to be very slow compared to other XML parsers. It's used to parse the koha-conf.xml, which is loaded on every page, taking ~600ms on my localhost, so this is a performance hit. These files currently use or reference XML::Simple: ./about.pl ./C4/Auth_with_ldap.pm ./C4/Context.pm ./C4/External/Amazon.pm ./C4/External/BakerTaylor.pm ./C4/External/Syndetics.pm ./C4/ILSDI/Services.pm ./C4/Installer/PerlDependencies.pm ./C4/Reports/Guided.pm ./C4/Search/PazPar2.pm ./C4/Search.pm ./C4/SIP/Sip/Configuration.pm ./C4/SIP/xmlparse.pl ./C4/XISBN.pm ./labels/label-create-xml.pl ./misc/bin/zebraqueue_daemon.pl ./misc/cronjobs/zebraqueue_start.pl ./opac/ilsdi.pl ./patroncards/create-pdf.pl ./patroncards/edit-layout.pl ./svc/authentication ./svc/bib ./svc/bib_profile ./svc/new_bib
XML::LibXML, XML::CompactTree::XS and XML::SAX::Simple are possible candidates... end output needs to be a reference to a hash of hashs ($context->{'var'}->{'othervar'})
My initial research has shown that a drop-in replacement for XML::Simple is hard to find. However, if we could parse the input as YAML instead of XML, then we could use YAML::Syck to load the file and return the same data structure. I've copied the <config> block of koha-conf.xml to koha-conf.yaml, and written a new subroutine to parse that, then modified C4::Context->new to detect the file type and use either XML or YAML according to the KOHA_CONF value. My VERY INITIAL tests indicate 2 orders of magnitude improvement in speed on that portion of code; from hundreds of milliseconds to 1 or 2. More research is needed to confirm these findings, but they're encouraging.
You should also look into bug 6193 which provides the speed increase from not having to parse the koha-conf.xml. And is ready to go right now
Created attachment 6618 [details] [review] Bug 7170: Remove XML::Simple usage in favour of YAML config file Enables support for a YAML koha-conf file.
Sorry for the delay in submitting this; it's been a long while to recover my footing post-KohaCon. A koha-conf.yaml file can be created out of the <config> tag of koha-conf.xml using this stylesheet: http://yaml.org/xml/xml2yaml.xsl This patch is proof-of-concept. If this is a direction we chose to go, then we need to put the YAML file into the Makefile, so it will be auto-generated along with koha-conf.xml.
I've tested ian script with misc/load_testing/benchmark_staff.pl, here are the results: (1st column with XML, 2nd column with YAML. Numbers are in ms, less is better) 23709|22453 30049|30749 27487|26463 52924|50355 35922|29679 33168|30388 The improvement is surprisingly low. Also note xsltproc xml2yaml.xsl koha-conf.xml returned nasty results, and I had to build the yaml manually, here is what it look like: db_scheme: mysql database: XXXXXXXX hostname: localhost port: 3306 user: XXXXXX pass: XXXXXXXXXX biblioserver: biblios biblioservershadow: 1 authorityserver: authorities authorityservershadow: 1 intranetdir: /home/paul/koha.dev/koha-community opacdir: /home/paul/koha.dev/koha-community/opac opachtdocs: /home/paul/koha.dev/koha-community/koha-tmpl/opac-tmpl intrahtdocs: /home/paul/koha.dev/koha-community/koha-tmpl/intranet-tmpl includes: /home/paul/koha.dev/koha-community/koha-tmpl/intranet-tmpl/prog/en/includes/ logdir: /home/paul/koha.dev/installs/lecannetdesmaures/var/log install_log: home/paul/koha.dev/installs/lecannetdesmaures/misc/koha-install-log useldapserver: 0 I also checked with NYTProf * with YAML = Profile of mainpage.pl for 1.97s (of 2.58s), executing 201727 statements and 52079 subroutine calls in 253 source files and 63 string evals. * with XML = Profile of mainpage.pl for 1.97s (of 2.60s), executing 201727 statements and 52079 subroutine calls in 253 source files and 63 string evals. still investigating, there must be something wrong...
(In reply to comment #6) > I also checked with NYTProf > * with YAML = Profile of mainpage.pl for 1.97s (of 2.58s), executing 201727 > statements and 52079 subroutine calls in 253 source files and 63 string evals. > * with XML = Profile of mainpage.pl for 1.97s (of 2.60s), executing 201727 > statements and 52079 subroutine calls in 253 source files and 63 string evals. > > still investigating, there must be something wrong... OK, I was testing twice with XML (forgot to update env variable) Here is the result: * with YAML: Profile of mainpage.pl for 1.92s (of 2.37s), executing 144420 statements and 33523 subroutine calls in 246 source files and 62 string evals. * with XML = Profile of mainpage.pl for 1.97s (of 2.60s), executing 201727 statements and 52079 subroutine calls in 253 source files and 63 string evals. it result in a *big* lowering of the number of statements executed, but not a big change in the duration of the page. If someone has an idea why... Looking at the details of NYTProf, I see: * 262ms is spent in XMLin (XML version) * 1.20ms is spent in YAML::Syck::LoadFile (YAML version) That's a 260ms difference, why don't we see it in the benchmark and/or in the total duration of mainpage reported by NYTProf
For the record: i've tested XML::SAX::Simple, just replacing the use XML::Simple by use XML::LibXML::Simple qw(XMLin); 1st you get an error : Unrecognised option: suppressempty at /home/paul/koha.dev/koha-community/C4/Context.pm line 231 If you remove the suppressemptry =>'' at line 231, you get: Bad arg length for Socket::pack_sockaddr_in, length is 0, should be 4 at /usr/lib/perl/5.12/Socket.pm line 217. I haven't investigated more
with koha-conf.xml being memcached now, I think this is no more interesting. Plus the patch does not work, it would need much more work to be managed during install. So marking "RESO WONTFIX"
I'm reopening this report, in it's boarder context of removing XML::Simple, rather than the specific context of a YAML config file. Obsoleting patch. XML::Simple is notoriously slow; if we've got to process any XML, we should be doing it with the most efficient and speedy parser we can get.
Hello Does someone still work on this bug? XML::Simple is still used by a lot of files. Any change that could improve performances would be a good thing! (And yes, I know Plack and Memcached ;-) ) Using Yaml instead of XML in some cases (particulary for Koha-conf) would also be a GREAT thing ( some libraries don't want to use Memcached). M. Saby Rennes 2 University
(In reply to comment #11) > Hello > Does someone still work on this bug? > XML::Simple is still used by a lot of files. I was actually thinking about working on this on my flight home tomorrow, actually. It depends how tired I am. In order to stay awake on flights I work on projects like this, that do not require any outside input. > Any change that could improve performances would be a good thing! (And yes, > I know Plack and Memcached ;-) ) Plack is not ready for production in my opinion, so I wasn't going to suggest it. Memcached is good, but not a silver bullet. For this, an mmap'd cache might be useful. I will experiment if I work on the problem. > Using Yaml instead of XML in some cases (particulary for Koha-conf) would > also be a GREAT thing ( some libraries don't want to use Memcached). If we can get drastically better performance using a different XML parser I think I might prefer that to switching to YAML, even if the other parser requires some massaging of the output data structure.
Also note that if you are not using XML::SAX::ExpatXS as your parser, you should be. You can do that by setting the environment variable XML_SIMPLE_PREFERRED_PARSER to 'XML::SAX::ExpatXS'.
(In reply to comment #12) > Plack is not ready for production in my opinion, so I wasn't going to > suggest it. Hi Jared, Why are you saying Plack is not ready for production ? we've it for the OPAC since 2 years. We're about to deploy it on staff interface at SAN-Ouest Provence. If there's some trouble I should expect, I prefer to know it ASAP !
(In reply to comment #14) > (In reply to comment #12) > > Plack is not ready for production in my opinion, so I wasn't going to > > suggest it. > > Hi Jared, > > Why are you saying Plack is not ready for production ? we've it for the OPAC > since 2 years. > We're about to deploy it on staff interface at SAN-Ouest Provence. If > there's some trouble I should expect, I prefer to know it ASAP ! As I have said many, many times, there are still a lot of problems with Plack, especially on the staff client. I patch them as soon as I find them, of course, but libraries expect all the pages to Just Work. The largest problem I am having with running the staff client under Plack at this point are to do with holds. Many times when I try to delete a hold either manually or by filling it I get a MySQL error, and the hold gets stuck. I haven't debugged the problem enough to file a bug report yet, so my workaround has just been deleting all holds every so often. There is also the problem with staging MARC files for importing (I get a "parseerror" dialog every so often, but the upload works, and I still haven't figured out why). On top of that, almost every time I test a patch that touches a script I haven't used before, I have run into Plack problems.
(In reply to comment #11) > Hello > Does someone still work on this bug? > XML::Simple is still used by a lot of files. > > Any change that could improve performances would be a good thing! (And yes, > I know Plack and Memcached ;-) ) > > Using Yaml instead of XML in some cases (particulary for Koha-conf) would > also be a GREAT thing ( some libraries don't want to use Memcached). I did a lot of testing, and my conclusions were as follows: 1. YAML is much faster than XML. 2. XML::Simple with $ENV{'XML_SIMPLE_PREFERRED_PARSER'} == 'XML::SAX::ExpatXS' is probably the best option for parsing the XML. XML::LibXML::Simple is a more-or-less drop-in replacement, but it's not appreciably faster.
Issue with XML::Simple still here in 16.05 the following files are still using it: automate_check-in-out.pl:use XML::Simple; C4/Reports/Guided.pm:use XML::Simple; C4/SIP/xmlparse.pl:use XML::Simple qw(:strict); C4/SIP/Sip/Configuration.pm:use XML::Simple qw(:strict); C4/XISBN.pm:use XML::Simple; C4/ILSDI/Services.pm:use XML::Simple; C4/ILSDI/Services.pm: use XML::Simple; C4/External/BakerTaylor.pm:use XML::Simple; C4/External/Syndetics.pm:use XML::Simple; C4/Search/PazPar2.pm:use XML::Simple; C4/Context.pm:use XML::Simple; C4/Search.pm:use XML::Simple; svc/new_bib:use XML::Simple; svc/bib:use XML::Simple; svc/bib_profile:use XML::Simple; svc/import_bib:use XML::Simple; svc/authentication:use XML::Simple; about.pl:use XML::Simple; misc/bin/connexion_import_daemon.pl:use XML::Simple; test_BZ8089.pl:use XML::Simple; opac/ilsdi.pl:use XML::Simple; patroncards/create-pdf.pl:use XML::Simple; patroncards/edit-layout.pl:use XML::Simple; labels/label-create-xml.pl:use XML::Simple;
I'd love to see XML::Simple disappear... As per the synopsis for XML::Simple itself says, "You really don't want to use this module in new code." http://search.cpan.org/~grantm/XML-Simple-2.22/lib/XML/Simple.pm#SYNOPSIS
I was looking in the svc ones and came up with this hackiness. sub build_xml_output { my ($xml_hash, $root_name) = @_; my @xml_tags = keys %{$xml_hash}; my @values = map { { 'key' => $_, 'value' => $xml_hash->{$_} } } @xml_tags; my $xml_values; foreach my $value (@values) { $xml_values->{$value->{key}} = $value->{value}; } my $dom = XML::LibXML::Document->new( '1.0', q{} ); $dom->setStandalone(1); my $root = $dom->createElement( $root_name ); $dom->setDocumentElement( $root ); foreach my $xml_tag (@xml_tags) { my $xml_value = $xml_values->{$xml_tag}; my $text = $dom->createTextNode( "\n " ); $root->appendChild($text); my $element = $dom->createElement( $xml_tag ); $root->appendChild($element); if ($xml_value =~ /\<datafield/ ) { my $parser = XML::LibXML->new(); my $fragment = $parser->parse_balanced_chunk( $xml_value ); $element->appendChild( $fragment ); } else { $text = $dom->createTextNode( $xml_value ); $element->appendTextNode($text); } } my $text = $dom->createTextNode( "\n" ); $root->appendChild($text); my $xml_output = $dom->toString(0); print $xml_output; } You can see why XML::Simple with its XMLout method is desirable. :)
> > You can see why XML::Simple with its XMLout method is desirable. :) Except I work with the Author of XML::Simple and as David has already pointed out, he recommends no one uses it.
(In reply to M. Tompsett from comment #19) > I was looking in the svc ones and came up with this hackiness. > > You can see why XML::Simple with its XMLout method is desirable. :) That does indeed appear very hacky. It doesn't need to be so hacky. I have quite a bit of experience with XML::LibXML, and it wouldn't be hard to write a solid predictable output function.
(In reply to Chris Cormack from comment #20) > > > > You can see why XML::Simple with its XMLout method is desirable. :) > > Except I work with the Author of XML::Simple and as David has already > pointed out, he recommends no one uses it. But this more than anything else.
I wonder if this might be a nice one to hit the 21.11 roadmap?
It looks like while some where fixed, others were added: C4/Auth_with_ldap.pm:XML::Simple() C4/External/BakerTaylor.pm C4/ILSDI/Services.pm C4/SIP/README C4/SIP/Sip/Configuration.pm C4/Search.pm C4/Search/PazPar2.pm C4/XISBN.pm Koha/SuggestionEngine/Plugin/LibrisSpellcheck.pm cpanfile labels/label-create-xml.pl misc/bin/connexion_import_daemon.pl misc/maintenance/generate_MARC21Languages.pl opac/ilsdi.pl patroncards/create-pdf.pl patroncards/edit-layout.pl svc/authentication svc/bib svc/bib_framework svc/bib_profile svc/import_bib svc/new_bib t/Biblio/TransformHtmlToXml.t t/db_dependent/OAI/Server.t t/db_dependent/www/auth_values_input_www.t t/db_dependent/www/batch.t t/db_dependent/www/history.t t/db_dependent/www/search_utf8.t