Bug 26848

Created attachment 112647 [details] [review]
Bug 26848: Fix Readonly dependency in cpanfile

- It is a required module. If it is missing, Koha will not start
- Set the minimum version to 2.00. While not strictly required, it's the
version that states that "Deprecation of Readonly::XS as a requirement
for fast, readonly scalars is complete", it is available in Debian
oldoldstable (jessie, 8), and Readonly::XS was not even needed with Perl
> 5.8, which allows us to:
- Remove dependency on Readonly::XS

Created attachment 114271 [details] [review]
Bug 26848: Add Readonly::XS 1.05 as required

hi Julian

i did a quick test with a modified eg/benchmark.pl script, which indicates that Readonly's performance is 10x with the Readonly::XS module installed

 https://metacpan.org/source/SANKO/Readonly-2.05/eg/benchmark.pl

so, i think it's still a good idea to install Readonly::XS - do you agree?


mason@xen1:~/g/cpan/readonly$ perl eg/benchmark2.pl
Benchmark: timing 10000000 iterations of rotie, rotie_xs,...
...
     rotie:  4 wallclock secs ( 4.19 usr +  0.00 sys =  4.19 CPU) @ 2386634.84/s (n=10000000)
  rotie_xs: -1 wallclock secs ( 0.38 usr +  0.00 sys =  0.38 CPU) @ 26315789.47/s (n=10000000)

Hi Mason,

What did you modify in the benchmark ?
I ran the non-modified benchmark with and without Readonly::XS installed and I saw no differences.

Also, on https://metacpan.org/pod/Readonly#Internals it is written:
> In the years since Readonly::XS was released, the then private internal
> methods have been exposed and can be used in pure perl. Similar modules were
> written to take advantage of this and a patch to Readonly was created. We no
> longer need to build and install another module to make Readonly useful on
> modern builds of perl.
> - You do not need to install Readonly::XS.
> - You should stop listing Readonly::XS as a dependency or expect it to be installed.
> - Stop testing the $Readonly::XSokay variable!

So I think Readonly::XS should be removed from our cpanfile.

Created attachment 114274 [details] [review]
Bug 26848: Fix Readonly dependency in cpanfile

- It is a required module. If it is missing, Koha will not start
- Set the minimum version to 2.00. While not strictly required, it's the
version that states that "Deprecation of Readonly::XS as a requirement
for fast, readonly scalars is complete", it is available in Debian
oldoldstable (jessie, 8), and Readonly::XS was not even needed with Perl
> 5.8, which allows us to:
- Remove dependency on Readonly::XS

Signed-off-by: Mason James <mtj@kohaaloha.com>

(In reply to Julian Maurice from comment #4)
> Hi Mason,
> 
> What did you modify in the benchmark ?
> I ran the non-modified benchmark with and without Readonly::XS installed and
> I saw no differences.
> 
> So I think Readonly::XS should be removed from our cpanfile.

hi Julian, 
i tested again and agree, so i will sign-off :)

Created attachment 115027 [details] [review]
Bug 26848: Fix Readonly dependency in cpanfile

- It is a required module. If it is missing, Koha will not start
- Set the minimum version to 2.00. While not strictly required, it's the
version that states that "Deprecation of Readonly::XS as a requirement
for fast, readonly scalars is complete", it is available in Debian
oldoldstable (jessie, 8), and Readonly::XS was not even needed with Perl
> 5.8, which allows us to:
- Remove dependency on Readonly::XS

Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Trivial fix.. also agree on the benchmark.. I can't replicate a speedup with ::XS.

Passing QA

Pushed to master for 21.05, thanks to everybody involved!

Pushed to 20.11.x for 20.11.02

Pushed to 20.05.x for 20.05.08

Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.