|
Description
Julian Maurice
2021-05-03 07:01:36 UTC
Created attachment 120374 [details] [review] Bug 28278: Add unit test for Koha::Config::read_from_file Created attachment 120375 [details] [review] Bug 28278: Improve $KOHA_CONF parsing speed by using XML::LibXML - Without the patch: $ hyperfine --warmup 1 \ 'perl -MKoha::Config -e "Koha::Config->read_from_file(\$ENV{KOHA_CONF}) for (1..1000)"' Time (mean ± σ): 3.585 s ± 0.018 s [User: 3.531 s, System: 0.049 s] Range (min … max): 3.547 s … 3.612 s 10 runs - With the patch: $ hyperfine --warmup 1 \ 'perl -MKoha::Config -e "Koha::Config->read_from_file(\$ENV{KOHA_CONF}) for (1..1000)"' Time (mean ± σ): 1.122 s ± 0.028 s [User: 1.104 s, System: 0.014 s] Range (min … max): 1.095 s … 1.189 s 10 runs Test plan: 1. Apply the first patch (the one with the unit tests) and make sure tests pass: `prove t/Koha/Config.t` 2. Apply the rest of the patches and verify that tests still pass: `prove t/Koha/Config.t`. Created attachment 120414 [details] [review] Bug 28278: Add unit test for Koha::Config::read_from_file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 120415 [details] [review] Bug 28278: Improve $KOHA_CONF parsing speed by using XML::LibXML - Without the patch: $ hyperfine --warmup 1 \ 'perl -MKoha::Config -e "Koha::Config->read_from_file(\$ENV{KOHA_CONF}) for (1..1000)"' Time (mean ± σ): 3.585 s ± 0.018 s [User: 3.531 s, System: 0.049 s] Range (min … max): 3.547 s … 3.612 s 10 runs - With the patch: $ hyperfine --warmup 1 \ 'perl -MKoha::Config -e "Koha::Config->read_from_file(\$ENV{KOHA_CONF}) for (1..1000)"' Time (mean ± σ): 1.122 s ± 0.028 s [User: 1.104 s, System: 0.014 s] Range (min … max): 1.095 s … 1.189 s 10 runs Test plan: 1. Apply the first patch (the one with the unit tests) and make sure tests pass: `prove t/Koha/Config.t` 2. Apply the rest of the patches and verify that tests still pass: `prove t/Koha/Config.t`. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Works :) How much is the gain then? 2.5s for 10000 runs? (In reply to Jonathan Druart from comment #6) > How much is the gain then? 2.5s for 10000 runs? It's for 1000 runs, so in the end I should gain something like 2.5ms at startup. It's not much, but still better, right ? :) Note that this was tested on my laptop with a i5-6440HQ CPU @ 2.60GHz The gain should be greater on slower hardware (just a guess; please test it on different hardware) Right, the "10 runs" confused me, but ofc it's an average. Without patch: Time (mean ± σ): 6.293 s ± 0.243 s [User: 6.181 s, System: 0.090 s] Range (min … max): 6.012 s … 6.852 s 10 runs With patch: Time (mean ± σ): 2.006 s ± 0.050 s [User: 1.937 s, System: 0.058 s] Range (min … max): 1.939 s … 2.092 s 10 runs i5-6200U CPU @ 2.30GHz Without patch Time (mean ± σ): 7.349 s ± 0.268 s [User: 6.451 s, System: 0.348 s] Range (min … max): 6.968 s … 7.775 s 10 runs With patch Time (mean ± σ): 2.377 s ± 0.104 s [User: 1.993 s, System: 0.148 s] Range (min … max): 2.266 s … 2.551 s 10 runs Run on a production virtual server. I'm happy with the result and the test is thorough... no regressions found. I'd love to see the subsequent bugs to split out the zebra config and possibly move away from XML.. but for now, even putting the performance improvement aside, this is a great step in the right direction for dropping our reliance on XML::Simple. Passing QA Created attachment 120455 [details] [review] Bug 28278: Add unit test for Koha::Config::read_from_file Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 120456 [details] [review] Bug 28278: Improve $KOHA_CONF parsing speed by using XML::LibXML - Without the patch: $ hyperfine --warmup 1 \ 'perl -MKoha::Config -e "Koha::Config->read_from_file(\$ENV{KOHA_CONF}) for (1..1000)"' Time (mean ± σ): 3.585 s ± 0.018 s [User: 3.531 s, System: 0.049 s] Range (min … max): 3.547 s … 3.612 s 10 runs - With the patch: $ hyperfine --warmup 1 \ 'perl -MKoha::Config -e "Koha::Config->read_from_file(\$ENV{KOHA_CONF}) for (1..1000)"' Time (mean ± σ): 1.122 s ± 0.028 s [User: 1.104 s, System: 0.014 s] Range (min … max): 1.095 s … 1.189 s 10 runs Test plan: 1. Apply the first patch (the one with the unit tests) and make sure tests pass: `prove t/Koha/Config.t` 2. Apply the rest of the patches and verify that tests still pass: `prove t/Koha/Config.t`. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 120666 [details] [review] Bug 28278: Uncomment and test access_dirs Pushed to master for 21.05, thanks to everybody involved! Enhancement not pushed to 20.11.x This patch broke ldap configuration - fix is submitted under Bug 28385 - ldap server configuration broken since migration from XML::Simple Looks like this introduced a problem for Shibboleth config too but I have a patch on bug 31393 for that. |