Fellow Developers, Last night, I stayed up late running circ/circulation.pl through NYTProf, to get an idea where we may be able to optimize circulation for speed. After much frustration (darn session IDs...), I was able to get a report. The results were... not exactly what I expected. It seems that much of the time spent running circ/circulation.pl is spent BEGINing the various C4 modules, multiple times. C4::Items, for example, is BEGuN 14 times in the execution of circ/circulation.pl. More stats: C4::Accounts - 8 BEGINs C4::Acquisition - 14 BEGINs C4::Auth - 14 BEGINs C4::Biblio - 14 BEGINs C4::Branch - 5 BEGINs C4::Circulation - 22 BEGINs C4::Context - 18 BEGINs C4::Dates - 12 BEGINs ... and so on. Even modules like C4::XSLT, C4::SMS, C4::Suggestions, and C4::Search::PazPar2 are BEGuN several times, without any calls to any of their subroutines. Some modules, it seems, don't have an END at the end... I was also able to get a Graphviz file out, showing all the package calls (folks may remember my keen interest in such things from KohaCon). Running it through sccmap to detect cycles, I found a very long one between several of the modules. About 8 packages long. All this indicate to me that we REALLY need to start thinking about imposing some kind of structure on C4. I'm thinking something with two-levels: Level 1 : Calls to the database, other direct interaction with stored data Level 2: Calls to Level 1 functions to manipulate data all our scripts would then be modified to call only Level 2, which would take care of talking to Level 1. At the worst, a particular Level 1 package would be BEGuN as many times as there were Level 2 packages BEGuN in the script (and we could certainly do better). It would eliminate all the cycles, as well. I can provide access to my report for anyone who needs it, or more details about how I obtained it. Whatever any of you need to keep this conversation going. This is a big deal, in my opinion, and we need to start detangling our code before too much long, or we're going to get seriously bogged down. Cheers, -Ian -- Ian Walls Lead Development Specialist ByWater Solutions Phone # (888) 900-8944 http://bywatersolutions.com ian.walls@bywatersolutions.com Twitter: @sekjal
On 30/03/11 15:47, Ian Walls wrote:
I was also able to get a Graphviz file out, showing all the package calls (folks may remember my keen interest in such things from KohaCon). Running it through sccmap to detect cycles, I found a very long one between several of the modules. About 8 packages long.
All this indicate to me that we REALLY need to start thinking about imposing some kind of structure on C4. I'm thinking something with two-levels:
Level 1 : Calls to the database, other direct interaction with stored data Level 2: Calls to Level 1 functions to manipulate data
all our scripts would then be modified to call only Level 2, which would take care of talking to Level 1. At the worst, a particular Level 1 package would be BEGuN as many times as there were Level 2 packages BEGuN in the script (and we could certainly do better). It would eliminate all the cycles, as well. Once we've abstracted the db to db layer, the opportunity to have a more logically arranged set of utility classes follows and from there you have choices of various paths to improve things. (and your approaching what most folk would consider the starting point for a modern web application)
I can provide access to my report for anyone who needs it, or more details about how I obtained it. I'd certainly like to see your figures and writing up the 'what you did' to get them on the wiki would be useful, and maybe encourage the rest of us to add some recipes.
Whatever any of you need to keep this conversation going. This is a big deal, in my opinion, and we need to start detangling our code before too much long, or we're going to get seriously bogged down. I couldn't agree more. The story is all bad the current big ball of mud makes testing difficult and doesn't abstract any complexity away (instead it adds to it)
Colin -- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 845 557 5634 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2 http://www.ptfs-europe.com
Le 30/03/2011 16:47, Ian Walls a écrit :
Fellow Developers,
Last night, I stayed up late running circ/circulation.pl <http://circulation.pl> through NYTProf, to get an idea where we may be able to optimize circulation for speed. After much frustration (darn session IDs...), I was able to get a report. The results were... not exactly what I expected.
It seems that much of the time spent running circ/circulation.pl <http://circulation.pl> is spent BEGINing the various C4 modules, multiple times. C4::Items, for example, is BEGuN 14 times in the execution of circ/circulation.pl <http://circulation.pl>. More stats:
C4::Accounts - 8 BEGINs C4::Acquisition - 14 BEGINs C4::Auth - 14 BEGINs C4::Biblio - 14 BEGINs C4::Branch - 5 BEGINs C4::Circulation - 22 BEGINs C4::Context - 18 BEGINs C4::Dates - 12 BEGINs
... and so on. Even modules like C4::XSLT, C4::SMS, C4::Suggestions, and C4::Search::PazPar2 are BEGuN several times, without any calls to any of their subroutines. Some modules, it seems, don't have an END at the end...
I was also able to get a Graphviz file out, showing all the package calls (folks may remember my keen interest in such things from KohaCon). Running it through sccmap to detect cycles, I found a very long one between several of the modules. About 8 packages long.
All this indicate to me that we REALLY need to start thinking about imposing some kind of structure on C4. I'm thinking something with two-levels:
Level 1 : Calls to the database, other direct interaction with stored data Level 2: Calls to Level 1 functions to manipulate data
all our scripts would then be modified to call only Level 2, which would take care of talking to Level 1. At the worst, a particular Level 1 package would be BEGuN as many times as there were Level 2 packages BEGuN in the script (and we could certainly do better). It would eliminate all the cycles, as well.
I can provide access to my report for anyone who needs it, or more details about how I obtained it. Whatever any of you need to keep this conversation going. This is a big deal, in my opinion, and we need to start detangling our code before too much long, or we're going to get seriously bogged down.
Cheers,
-Ian
Thanks Ian for that investigation. I am really interested in your figures and ways to investigate. What you are trying to say meets the concern of many of us. That kind of Begin calls would be a killer for Data Persistence. But It seems that when you preload the code, things are behaving not so badly. I tried to launch starman a prod.psgi in order to test the Plack stuff and it behaved pretty good. I also proposed that we could use a kind of "Controller" Modules in order to externalise those CanItemBe.... Can.... this Action be taken... This often leads to use modules in a strange way : think of Circulation and reservation for instance. I had proposed to get those subroutines out of those modules. But it would add to the list of modules we import at the view level. In my opinion, the simpler we get, the more powerful and flexible we will be. Having simple 'objects' which creates/adds/updates/display/delete and are searchable with simple APIs, and a way to make use of those objects no matter the backend (DBI, Storable, YAML) could be achieved... But not by one company on its own. It would need coordination and cooperation. Shifting to OO design would imply some overload that maybe not everyone agrees on... And this would need some Data persistance layer in order to cope with that. But yes, there is place and need for a refactoring... that we can achieve if we all join forces. And if we have a big picture and a kind of schedule, and also exemples of how things should be done then it would be much easier for ppl to collaborate. This won't be achieved in 6 months, maybe not even in a year... But i think it is worth... -- Henri-Damien LAURENT
Hello, Ian. Disentangling these dependency cycles is essential for Koha's development to continue on in a meaningful way. As is, changing one small section of code often requires changing a very similar chunk of code in five other areas (if you know about all five of those other areas, which you probably don't, which means you've introduced action-at-a-distance bugs). The approach of separating the DB schema layer out from a model layer is a solid first step. Plus, the deep entangling makes reliable testing extremely difficult, meaning that the AAD discrepancies are difficult to detect in an automated fashion. Separating out the view and controller pieces is also essential for testing. However, this step by itself is only going to marginally optimize for execution time. It will help inasmuch as it may reduce the number of modules that get loaded for any given operation, and that's a boon. But each module only ever gets loaded and its BEGIN block processed once per script execution cycle, regardless of how many times it is "used." At the same time, many of the efficiency issues are the product of processing the same data a thousand times instead of once, because each model-layer subroutine makes direct calls to the database instead of normalizing those accesses into a schema management layer that can uniformly cache (and un-cache!) data. Ultimately, migrating from CGI to persistence is the big step that needs to be taken for improving speed, but that step cannot be taken until the API is disentangled, making execution more predictable. Regards, Clay 2011/3/30 Ian Walls <ian.walls@bywatersolutions.com>
Fellow Developers,
Last night, I stayed up late running circ/circulation.pl through NYTProf, to get an idea where we may be able to optimize circulation for speed. After much frustration (darn session IDs...), I was able to get a report. The results were... not exactly what I expected.
It seems that much of the time spent running circ/circulation.pl is spent BEGINing the various C4 modules, multiple times. C4::Items, for example, is BEGuN 14 times in the execution of circ/circulation.pl. More stats:
C4::Accounts - 8 BEGINs C4::Acquisition - 14 BEGINs C4::Auth - 14 BEGINs C4::Biblio - 14 BEGINs C4::Branch - 5 BEGINs C4::Circulation - 22 BEGINs C4::Context - 18 BEGINs C4::Dates - 12 BEGINs
... and so on. Even modules like C4::XSLT, C4::SMS, C4::Suggestions, and C4::Search::PazPar2 are BEGuN several times, without any calls to any of their subroutines. Some modules, it seems, don't have an END at the end...
I was also able to get a Graphviz file out, showing all the package calls (folks may remember my keen interest in such things from KohaCon). Running it through sccmap to detect cycles, I found a very long one between several of the modules. About 8 packages long.
All this indicate to me that we REALLY need to start thinking about imposing some kind of structure on C4. I'm thinking something with two-levels:
Level 1 : Calls to the database, other direct interaction with stored data Level 2: Calls to Level 1 functions to manipulate data
all our scripts would then be modified to call only Level 2, which would take care of talking to Level 1. At the worst, a particular Level 1 package would be BEGuN as many times as there were Level 2 packages BEGuN in the script (and we could certainly do better). It would eliminate all the cycles, as well.
I can provide access to my report for anyone who needs it, or more details about how I obtained it. Whatever any of you need to keep this conversation going. This is a big deal, in my opinion, and we need to start detangling our code before too much long, or we're going to get seriously bogged down.
Cheers,
-Ian
-- Ian Walls Lead Development Specialist ByWater Solutions Phone # (888) 900-8944 http://bywatersolutions.com ian.walls@bywatersolutions.com Twitter: @sekjal
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
If anyone wants to try their hand at breaking this, here's the big cycle I mentioned (in .dot format) digraph cluster_9 { graph [overlap=false]; node [shape=doublecircle]; "C4::Members" -> "C4::Members"; "C4::Members" -> "C4::Reserves"; "C4::Items" -> "C4::Items"; "C4::Items" -> "C4::Acquisition"; "C4::Letters" -> "C4::Members"; "C4::Letters" -> "C4::Letters"; "C4::XSLT" -> "C4::Items"; "C4::Heading" -> "C4::Search"; "C4::Acquisition" -> "C4::Suggestions"; "C4::Suggestions" -> "C4::Letters"; "C4::Reserves" -> "C4::Heading"; "C4::Reserves" -> "C4::Reserves"; "C4::Search" -> "C4::XSLT"; } I commented out the connection from Items to Acquistions, but the overall reduction was quite minimal. Perhaps the connection from Reserves to Heading could be snipped... not sure why Holds would need to factor in Authorities... Big project, for sure. -Ian On Wed, Mar 30, 2011 at 1:17 PM, Clay Fouts <cfouts@liblime.com> wrote:
Hello, Ian.
Disentangling these dependency cycles is essential for Koha's development to continue on in a meaningful way. As is, changing one small section of code often requires changing a very similar chunk of code in five other areas (if you know about all five of those other areas, which you probably don't, which means you've introduced action-at-a-distance bugs). The approach of separating the DB schema layer out from a model layer is a solid first step. Plus, the deep entangling makes reliable testing extremely difficult, meaning that the AAD discrepancies are difficult to detect in an automated fashion. Separating out the view and controller pieces is also essential for testing.
However, this step by itself is only going to marginally optimize for execution time. It will help inasmuch as it may reduce the number of modules that get loaded for any given operation, and that's a boon. But each module only ever gets loaded and its BEGIN block processed once per script execution cycle, regardless of how many times it is "used." At the same time, many of the efficiency issues are the product of processing the same data a thousand times instead of once, because each model-layer subroutine makes direct calls to the database instead of normalizing those accesses into a schema management layer that can uniformly cache (and un-cache!) data.
Ultimately, migrating from CGI to persistence is the big step that needs to be taken for improving speed, but that step cannot be taken until the API is disentangled, making execution more predictable.
Regards, Clay
2011/3/30 Ian Walls <ian.walls@bywatersolutions.com>
Fellow Developers,
Last night, I stayed up late running circ/circulation.pl through NYTProf, to get an idea where we may be able to optimize circulation for speed. After much frustration (darn session IDs...), I was able to get a report. The results were... not exactly what I expected.
It seems that much of the time spent running circ/circulation.pl is spent BEGINing the various C4 modules, multiple times. C4::Items, for example, is BEGuN 14 times in the execution of circ/circulation.pl. More stats:
C4::Accounts - 8 BEGINs C4::Acquisition - 14 BEGINs C4::Auth - 14 BEGINs C4::Biblio - 14 BEGINs C4::Branch - 5 BEGINs C4::Circulation - 22 BEGINs C4::Context - 18 BEGINs C4::Dates - 12 BEGINs
... and so on. Even modules like C4::XSLT, C4::SMS, C4::Suggestions, and C4::Search::PazPar2 are BEGuN several times, without any calls to any of their subroutines. Some modules, it seems, don't have an END at the end...
I was also able to get a Graphviz file out, showing all the package calls (folks may remember my keen interest in such things from KohaCon). Running it through sccmap to detect cycles, I found a very long one between several of the modules. About 8 packages long.
All this indicate to me that we REALLY need to start thinking about imposing some kind of structure on C4. I'm thinking something with two-levels:
Level 1 : Calls to the database, other direct interaction with stored data Level 2: Calls to Level 1 functions to manipulate data
all our scripts would then be modified to call only Level 2, which would take care of talking to Level 1. At the worst, a particular Level 1 package would be BEGuN as many times as there were Level 2 packages BEGuN in the script (and we could certainly do better). It would eliminate all the cycles, as well.
I can provide access to my report for anyone who needs it, or more details about how I obtained it. Whatever any of you need to keep this conversation going. This is a big deal, in my opinion, and we need to start detangling our code before too much long, or we're going to get seriously bogged down.
Cheers,
-Ian
-- Ian Walls Lead Development Specialist ByWater Solutions Phone # (888) 900-8944 http://bywatersolutions.com ian.walls@bywatersolutions.com Twitter: @sekjal
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Ian Walls Lead Development Specialist ByWater Solutions Phone # (888) 900-8944 http://bywatersolutions.com ian.walls@bywatersolutions.com Twitter: @sekjal
Hi, 2011/3/30 Ian Walls <ian.walls@bywatersolutions.com>:
All this indicate to me that we REALLY need to start thinking about imposing some kind of structure on C4. I'm thinking something with two-levels:
Level 1 : Calls to the database, other direct interaction with stored data Level 2: Calls to Level 1 functions to manipulate data
Thanks for doing the analysis. Regarding the overall project of restructuring C4, I suggest a slightly different way of looking at it: *not* restructuring C4 as such, but using it as a springboard for its successor. In other words, we'd still be creating a better structure, just in a different namespace. Let's pick a name randomly for that namespace. I reach into my hat and pull out ... "Koha". Fancy that. :) The Koha namespace would be for Perl modules that meet the following mandatory conditions: - usable by mod_perl (or its equivalent) - export the minimum number of routines necessary - follow stricture, warnings, documentation, and test case standards - separating data access methods from business logic and preferably the following optional conditions: - purely OO Obviously, the conditions and whether they would be mandatory or optional is a topic for discussion. Modules in C4 could use (and would be encouraged to use) routines in the Koha namespace. Modules in Koha could not in general use C4 modules; any C4 module that was safe to be depended on by a Koha module would be a candidate for being renamed to Koha. The advantage of carving out a new namespace is that it doesn't require that we refactor the entirety of C4 to support persistance or to untangle the dependency tree. Instead, the only C4 code we would have to reimplement for the Koha namespace right away would be authentication, basic session management, and basic output. That, along with data access using DBIx::Class, would allow us to start using mod_perl right away for specific functions. One example of something that would then be easy to transform to a service running under mod_perl would be the unapi and OAI-PMH services. As functionality gets detangled from C4 and moved into Koha, we can increase the number of services running under mod_perl while still keeping the CGI scripts that still depend on C4. Regards, Galen -- Galen Charlton gmcharlt@gmail.com
2011/3/31 Galen Charlton <gmcharlt@gmail.com>
Hi,
2011/3/30 Ian Walls <ian.walls@bywatersolutions.com>:
All this indicate to me that we REALLY need to start thinking about imposing some kind of structure on C4. I'm thinking something with two-levels:
Level 1 : Calls to the database, other direct interaction with stored data Level 2: Calls to Level 1 functions to manipulate data
Thanks for doing the analysis. Regarding the overall project of restructuring C4, I suggest a slightly different way of looking at it: *not* restructuring C4 as such, but using it as a springboard for its successor.
In other words, we'd still be creating a better structure, just in a different namespace. Let's pick a name randomly for that namespace. I reach into my hat and pull out ... "Koha". Fancy that. :)
The Koha namespace would be for Perl modules that meet the following mandatory conditions:
- usable by mod_perl (or its equivalent) - export the minimum number of routines necessary - follow stricture, warnings, documentation, and test case standards - separating data access methods from business logic
and preferably the following optional conditions:
- purely OO
Obviously, the conditions and whether they would be mandatory or optional is a topic for discussion.
Modules in C4 could use (and would be encouraged to use) routines in the Koha namespace. Modules in Koha could not in general use C4 modules; any C4 module that was safe to be depended on by a Koha module would be a candidate for being renamed to Koha.
The advantage of carving out a new namespace is that it doesn't require that we refactor the entirety of C4 to support persistance or to untangle the dependency tree. Instead, the only C4 code we would have to reimplement for the Koha namespace right away would be authentication, basic session management, and basic output.
About authentication and basic session management, I would advise to use Plack::Middlewares (Plack::Middleware::Authen) instead of re building our own modules. This would allow to be much more adaptive. There is also a middleware for session management iirc.
That, along with data access using DBIx::Class, would allow us to start using mod_perl right away for specific functions. One example of something that would then be easy to transform to a service running under mod_perl would be the unapi and OAI-PMH services. As functionality gets detangled from C4 and moved into Koha, we can increase the number of services running under mod_perl while still keeping the CGI scripts that still depend on C4.
Regards,
Galen
-- -- Henri-Damien LAURENT BibLibre SARL http://www.biblibre.com Expert en Logiciels Libres pour l'info-doc tel : +33 4 67 65 75 50
On 31 March 2011 16:51, Galen Charlton <gmcharlt@gmail.com> wrote:
Hi,
2011/3/30 Ian Walls <ian.walls@bywatersolutions.com>:
All this indicate to me that we REALLY need to start thinking about imposing some kind of structure on C4. I'm thinking something with two-levels:
Level 1 : Calls to the database, other direct interaction with stored data Level 2: Calls to Level 1 functions to manipulate data
Thanks for doing the analysis. Regarding the overall project of restructuring C4, I suggest a slightly different way of looking at it: *not* restructuring C4 as such, but using it as a springboard for its successor.
In other words, we'd still be creating a better structure, just in a different namespace. Let's pick a name randomly for that namespace. I reach into my hat and pull out ... "Koha". Fancy that. :)
The Koha namespace would be for Perl modules that meet the following mandatory conditions:
- usable by mod_perl (or its equivalent) - export the minimum number of routines necessary - follow stricture, warnings, documentation, and test case standards - separating data access methods from business logic
and preferably the following optional conditions:
- purely OO
Obviously, the conditions and whether they would be mandatory or optional is a topic for discussion.
Modules in C4 could use (and would be encouraged to use) routines in the Koha namespace. Modules in Koha could not in general use C4 modules; any C4 module that was safe to be depended on by a Koha module would be a candidate for being renamed to Koha.
The advantage of carving out a new namespace is that it doesn't require that we refactor the entirety of C4 to support persistance or to untangle the dependency tree. Instead, the only C4 code we would have to reimplement for the Koha namespace right away would be authentication, basic session management, and basic output. That, along with data access using DBIx::Class, would allow us to start using mod_perl right away for specific functions. One example of something that would then be easy to transform to a service running under mod_perl would be the unapi and OAI-PMH services. As functionality gets detangled from C4 and moved into Koha, we can increase the number of services running under mod_perl while still keeping the CGI scripts that still depend on C4.
+1 From me I think this is the way to go. Chris
Galen Charlton schreef op wo 30-03-2011 om 20:51 [-0700]:
Thanks for doing the analysis. Regarding the overall project of restructuring C4, I suggest a slightly different way of looking at it: *not* restructuring C4 as such, but using it as a springboard for its successor.
I approve of this idea, and am starting it with my accounts rewrite. I'll create a Koha/Accounts.pm and am in the process of pulling all the direct database access to the accounts tables out of other modules and putting the required functionality into the accounts module. Additionally, do we want to split things up within the Koha namespace, for example database-accessing code vs. the more business logicy stuff? My personal inclination is to not do that, and it wouldn't be a terrible refactoring to do at a later stage if it were required. Oh, something else that's recommended that I tend to do is not export anything by default, requiring the module to be included like: use C4::Accounts qw/ get_users_balance /; mostly this prevents surprising things getting into your namespace. This is more a style suggestion though. -- Robin Sheat Catalyst IT Ltd. ✆ +64 4 803 2204 GPG: 5957 6D23 8B16 EFAB FEF8 7175 14D3 6485 A99C EB6D
Oh, something else that's recommended that I tend to do is not export anything by default, requiring the module to be included like: use C4::Accounts qw/ get_users_balance /; mostly this prevents surprising things getting into your namespace. This is more a style suggestion though. More than style I think it generally accepted as good practice and should be encouraged. One of the downsides of just using default export
On 06/04/11 05:11, Robin Sheat wrote: lists apart from general namespace pollution is it gets hard to know what you are using from module X (and sometimes on further investigation it turns out to be nothing!) Colin -- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 845 557 5634 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2 http://www.ptfs-europe.com
Hi, On Wed, Apr 6, 2011 at 12:11 AM, Robin Sheat <robin@catalyst.net.nz> wrote:
Additionally, do we want to split things up within the Koha namespace, for example database-accessing code vs. the more business logicy stuff? My personal inclination is to not do that, and it wouldn't be a terrible refactoring to do at a later stage if it were required.
I'd prefer a separation between business logic and data access, actually. Perhaps # data access - though the choice of naming may better follow whatever pattern DBIx::Class imposes: Koha::DB::biblio Koha::DB::accounts # business logic Koha::Acounts Koha::Cataloging Koha::Cataloging::Biblio or the like. Obviously we could debate the hierarchy ad infinitum, but as long as names are well-chosen and we don't lump unrelated things together, we can run with what works.
Oh, something else that's recommended that I tend to do is not export anything by default, requiring the module to be included like: use C4::Accounts qw/ get_users_balance /; mostly this prevents surprising things getting into your namespace. This is more a style suggestion though.
I agree with Colin that this should be more than just a style suggestion. We shouldn't include more than the bare minimum in @EXPORT; @EXPORT_OK is to be preferred. Even better would be an object oriented design. Regards, Galen -- Galen Charlton gmcharlt@gmail.com
I also would like the separation of business logic from data access. I'm seeing several tiers in the structure: Template Script Business Logic library Data Access library Data structure We can then have multiple templates to meet the needs of the library. Bonus points for making it fully customizable in by the librarians. For third party integration, like SIP2 or NCIP, we would probably need separate script level from the normal routines. This wouldn't be a problem if core functions were abstracted into the Business Logic library level. That level would in turn call the data access level, which we could set up to handle multiple backends (MySQL, Postgres, etc) Separately, in addition to only exporting the functions needed, what about moving the "require C4::whatever;" lines out of the BEGIN{} block and into the subroutine that needs use of whatever's functions? Right now, we load all the modules at the beginning, even if only a single subroutine makes use of that module. If the script that uses the module only exports a single function, and it's not the subroutine that's being used, lots of time is wasted loading extraneous code. For example, in my profile of circ/circulation.pl, I found that C4::Budgets, C4::Category, C4::ClassSource, C4::Heading, C4::Heading::MARC21, C4::ItemType, C4::Message, C4::Print, C4::Search, C4::Search::PazPar2, C4::Serials, C4::SMS, C4::SQLHelper, C4::Suggestions, C4::VirtualShelves and C4::XSLT were all loaded, but none of their subroutines used. This may only save a few microseconds, but perhaps the simplification would help. Cheers, -Ian On Wed, Apr 6, 2011 at 8:30 AM, Galen Charlton <gmcharlt@gmail.com> wrote:
Hi,
On Wed, Apr 6, 2011 at 12:11 AM, Robin Sheat <robin@catalyst.net.nz> wrote:
Additionally, do we want to split things up within the Koha namespace, for example database-accessing code vs. the more business logicy stuff? My personal inclination is to not do that, and it wouldn't be a terrible refactoring to do at a later stage if it were required.
I'd prefer a separation between business logic and data access, actually. Perhaps
# data access - though the choice of naming may better follow whatever pattern DBIx::Class imposes: Koha::DB::biblio Koha::DB::accounts
# business logic Koha::Acounts Koha::Cataloging Koha::Cataloging::Biblio
or the like. Obviously we could debate the hierarchy ad infinitum, but as long as names are well-chosen and we don't lump unrelated things together, we can run with what works.
Oh, something else that's recommended that I tend to do is not export anything by default, requiring the module to be included like: use C4::Accounts qw/ get_users_balance /; mostly this prevents surprising things getting into your namespace. This is more a style suggestion though.
I agree with Colin that this should be more than just a style suggestion. We shouldn't include more than the bare minimum in @EXPORT; @EXPORT_OK is to be preferred.
Even better would be an object oriented design.
Regards,
Galen -- Galen Charlton gmcharlt@gmail.com
-- Ian Walls Lead Development Specialist ByWater Solutions ALA Booth 732 Phone # (888) 900-8944 http://bywatersolutions.com ian.walls@bywatersolutions.com Twitter: @sekjal
Hi, On Wed, Apr 6, 2011 at 9:12 AM, Ian Walls <ian.walls@bywatersolutions.com> wrote:
I also would like the separation of business logic from data access. I'm seeing several tiers in the structure:
Template Script Business Logic library Data Access library Data structure
We can then have multiple templates to meet the needs of the library. Bonus points for making it fully customizable in by the librarians.
But let's be careful not to get too far ahead of ourselves (regarding fully-customizable templating, which is not strictly an API issue. And besides, the templates are already fully customizable, albeit with a higher bar to entry than is desirable). But in general, I agree: presentation should be separated from business logic should be separated from data access. Regards, Galen -- Galen Charlton gmcharlt@gmail.com
On Wed, Apr 6, 2011 at 10:21 AM, Galen Charlton <gmcharlt@gmail.com> wrote:
Hi,
On Wed, Apr 6, 2011 at 9:12 AM, Ian Walls <ian.walls@bywatersolutions.com> wrote:
I also would like the separation of business logic from data access. I'm seeing several tiers in the structure:
Template Script Business Logic library Data Access library Data structure
We can then have multiple templates to meet the needs of the library. Bonus points for making it fully customizable in by the librarians.
But let's be careful not to get too far ahead of ourselves (regarding fully-customizable templating, which is not strictly an API issue. And besides, the templates are already fully customizable, albeit with a higher bar to entry than is desirable). But in general, I agree: presentation should be separated from business logic should be separated from data access.
This sounds like an MVC approach. Aren't we just coming full circle with earlier discussions about refactoring Koha around a framework like Catalyst, Moose, etc.? http://lists.koha-community.org/pipermail/koha-devel/2009-January/031655.htm... http://lists.koha-community.org/pipermail/koha-devel/2010-June/034106.html http://permalink.gmane.org/gmane.education.libraries.koha.devel/5218 Is this the time to bring these and other threads back to the table and move forward with a concrete decision rather than going at it piecemeal? Kind Regards, Chris
Hi, On Wed, Apr 6, 2011 at 1:43 PM, Chris Nighswonger <cnighswonger@foundations.edu> wrote:
Is this the time to bring these and other threads back to the table and move forward with a concrete decision rather than going at it piecemeal?
Given Robin's announcement that he's adopting the Koha namespace for his accounts rewrite, I think we actually have, or are about to have, something more concrete. Obviously, we could turn this into a discussion of adopting a particular framework, but given that those discussions have historically not been productive in terms of producing much code, I think we're best off just setting some broad guidelines, seeing what emerges in upcoming patches, *then* deciding if we want to adopt a particular framework. Is this putting the cart before the horse? Perhaps, but I think we're better off coding some prototypes instead of risking endless discussion. To make this more explicit: we have time before coding for 3.6 *really* kicks off to do some experimentation collectively. Regards, Galen -- Galen Charlton gmcharlt@gmail.com
* Galen Charlton (gmcharlt@gmail.com) wrote:
Hi,
On Wed, Apr 6, 2011 at 1:43 PM, Chris Nighswonger <cnighswonger@foundations.edu> wrote:
Is this the time to bring these and other threads back to the table and move forward with a concrete decision rather than going at it piecemeal?
Given Robin's announcement that he's adopting the Koha namespace for his accounts rewrite, I think we actually have, or are about to have, something more concrete. Obviously, we could turn this into a discussion of adopting a particular framework, but given that those discussions have historically not been productive in terms of producing much code, I think we're best off just setting some broad guidelines, seeing what emerges in upcoming patches, *then* deciding if we want to adopt a particular framework. Is this putting the cart before the horse? Perhaps, but I think we're better off coding some prototypes instead of risking endless discussion.
+1 Chris -- Chris Cormack Catalyst IT Ltd. +64 4 803 2238 PO Box 11-053, Manners St, Wellington 6142, New Zealand
Galen Charlton schreef op wo 06-04-2011 om 13:52 [-0400]:
Perhaps, but I think we're better off coding some prototypes instead of risking endless discussion.
Definitely. Additionally, if we change our minds, refactoring three or four modules isn't a big task at all. -- Robin Sheat Catalyst IT Ltd. ✆ +64 4 803 2204 GPG: 5957 6D23 8B16 EFAB FEF8 7175 14D3 6485 A99C EB6D
On 06/04/11 13:30, Galen Charlton wrote:
Hi,
On Wed, Apr 6, 2011 at 12:11 AM, Robin Sheat <robin@catalyst.net.nz> wrote:
Additionally, do we want to split things up within the Koha namespace, for example database-accessing code vs. the more business logicy stuff? My personal inclination is to not do that, and it wouldn't be a terrible refactoring to do at a later stage if it were required.
I'd prefer a separation between business logic and data access, actually. Perhaps
# data access - though the choice of naming may better follow whatever pattern DBIx::Class imposes:
If anyone wants to experiment with DBIx::Class using DBIx::Class::Schema::Loader gives you a quick way to start: from the kohaclone directory you use the command dbicdump -o dump_directory=. Koha::Schema dbi:mysql:dbname=koha kohaadminuser password This writes a Koha::Schema class and a Koha::Schema::Results::X class for every table in the database and you can regenerate it the same way to reflect db changes (but it preserves any custom code you add) Colin -- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 845 557 5634 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2 http://www.ptfs-europe.com
Le 30/03/2011 16:47, Ian Walls a écrit :
Fellow Developers, Hello everybody, Last night, I stayed up late running circ/circulation.pl <http://circulation.pl> through NYTProf, to get an idea where we may be able to optimize circulation for speed. After much frustration (darn session IDs...), I was able to get a report. The results were... not exactly what I expected.
For those who have used or developed on Koha 2.x, you probably have noticed that Koha 2.x was much more slower than Koha 3.x So, yesterday and tonight i've taken my laptop and made a lot of testings. First of all, i'll thank forever ... The New York Times ... Why will you tell me ? Because of http://search.cpan.org/~adamk/Aspect-Library-NYTProf-1.00/lib/Aspect/Library... This tool is really amazing. You run perl -d:NYTProf mainpage.pl ; nytprofhtml --open And you get a highly detailled HTML report with the time spent in each line/sub/module to run mainpage.pl. Here is the result : http://depot.biblibre.com/ppoulain/nytprof.ini/ (the .ini is to keep track of the perf before anything optimized) Note : tests done on my laptop, that is not a server. 1st test with master: Profile of mainpage.pl for 1.73s (of 2.18s), executing 155108 statements and 41041 subroutine calls in 249 source files and 64 string evals. I have investigated to find the longest timings & if we can do something. (note All the following improvement have been done incrementally) First of all, C4::Context and C4::Koha are loaded on everypage, so it's worth investigating == C4::Context == * spent 185ms just reading the config file. Ouch ! that's almost 10% of the time !!! (http://depot.biblibre.com/ppoulain/nytprof.ini/C4-Context-pm-13-line.html#23...) I tried to hardcode the hashref instead of XMLin-ing the config file, the results are astonishing : after = Profile of mainpage.pl for 1.59s (of 1.93s), executing 117399 statements and 26939 subroutine calls in 241 source files and 63 string evals. => PROPOSAL = add a YAML version of the XML config file that we could use in // => PROPOSAL 2 = memcache the config file (hint : the config file in koha 2 was not xml !) * C4::Context, sub db_scheme2dbi, the i flag in /mysql/i cost 84ms !!! with /i, the duration is 84.4ms, without, it's 10µs !!! after = Profile of mainpage.pl for 1.55s (of 1.91s), executing 117398 statements and 26938 subroutine calls in 241 source files and 63 string evals. => PROPOSAL = just 'return "mysql"' at the beginning of this sub, as everything else is not working anyway ! == C4::Languages == * C4::Languages::getAllLanguages 43ms => heavily under optimized mysql query = returns all languages from language_subtag_registry and language_descriptions even if only a few localisations are installed. * C4::Languages::_build_languages_arrayref 15ms => same as for getAllLanguages I know there is a patch (from hdl) pending to improve this sub, i haven't tested it yet (http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6000) (hint : this sub appeared in 3.0, was not in 2.2 !) == Other packages == * C4::Koha.pm 15ms for use Business::ISBN, that is used only in 1 sub used only in a few pages (and none if no cover images are enabled unless i'm wrong) * C4::Letters use MIME::Lite spend 26ms, that are useless 90% of the time (why is C4::Letters loaded ! answer = because C4::Reserves is loaded by Items.pm that is loaded by Biblio.pm that is loaded by AuthoritiesMarc.pm that is loaded (uselessly) by mainpage.pl * CGI/Session/Driver/mysql.pm needs 79ms i've tested by switching to driver::File::Serializer (dropped to 21ms). this seems to work, but probably requires more testing than what I did * Date::Manip needs requires 46ms and is useless most of the time. after = Profile of mainpage.pl for 1.31s (of 1.60s), executing 103643 statements and 25380 subroutine calls in 215 source files and 55 string evals. => PROPOSAL = WHERE it's interesting we could replace the use XXXX by a require XXXX just before the lines that need the package. The require is loaded at run tim, so, when unneeded, the package won't be loaded == mainpage.pl == Mainpage.pl load some authorities, we do nothing with it. Commenting the use C4::Authorities and all related code results in: after = Profile of mainpage.pl for 1.27s (of 1.57s), executing 103000 statements and 25194 subroutine calls in 212 source files and 55 string evals. == Packages nested == * I also noticed that Auth.pm loads Members.pm (to retrieve patron informations and display them on the top right). Members.pm loads Reserves that loads Biblio.pm, Circulation.pm and Items.pm All those packages are the biggest to load. I've written a script, that display, for each file in C4 which package is loaded and which sub in the package loaded are used. It shows, for example, that there is a "use Koha.pm;" that is useless in AuthoritiesMarc.pm It also shows which sub are used, and how many times. For example, Circulation.pm calls "use C4::Biblio" and : In Circulation.pm the Biblio.pm, sub GetBiblioItemData is used 1 times In Circulation.pm the Biblio.pm, sub GetBiblioFromItemNumber is used 4 times That will be handy to find what can be cleaned & removed. I've attached the script and the result. If you want to run it yourself, just export PERL5LIB, KOHA_CONF, put the script in $KOHA directory, and run it ! == Other == * packages that uses the biggest amount of CPU are not C4 packages : * utf8_heavy-pl is the most cpu consuming thing : 132ms, I don't know if we can do something * Template::Parser needs 123ms, not sure we can do something * all XML stuff is highly consuming = loading MARC::File::XML requires 47ms in Biblio.pm, XML::Sax::Base require 40ms to be loaded => definetly, the best way to reduce CPU time is by pre-loading things, we already knew that == Next step == * i'll file a bug for the various improvements that can be made easily * i'll continue investigate & report my findings I'm aware it's a 1st step, and the long-term road is to reach data&code persistency. But cleaning what we already have may ease the next big step. And any speed improvement is a good thing, isn't it ! HTH -- Paul POULAIN http://www.biblibre.com Expert en Logiciels Libres pour l'info-doc Tel : (33) 4 91 81 35 08
Le 30/03/2011 16:47, Ian Walls a écrit :
Fellow Developers,
It seems that much of the time spent running circ/circulation.pl <http://circulation.pl> is spent BEGINing the various C4 modules, multiple times. C4::Items, for example, is BEGuN 14 times in the execution of circ/circulation.pl <http://circulation.pl>. More stats:
C4::Accounts - 8 BEGINs C4::Acquisition - 14 BEGINs C4::Auth - 14 BEGINs C4::Biblio - 14 BEGINs C4::Branch - 5 BEGINs C4::Circulation - 22 BEGINs C4::Context - 18 BEGINs C4::Dates - 12 BEGINs Ian, where do you see the number of BEGINs and the time spent on it ? I can't find it on NYTProf report.
-- Paul POULAIN http://www.biblibre.com Expert en Logiciels Libres pour l'info-doc Tel : (33) 4 91 81 35 08
(this mail has been sent 10 hours ago, seems it was too large. I send it again without attached docs) Le 30/03/2011 16:47, Ian Walls a écrit :
Fellow Developers, Hello everybody, Last night, I stayed up late running circ/circulation.pl <http://circulation.pl> through NYTProf, to get an idea where we may be able to optimize circulation for speed. After much frustration (darn session IDs...), I was able to get a report. The results were... not exactly what I expected.
For those who have used or developed on Koha 2.x, you probably have noticed that Koha 2.x was much more slower than Koha 3.x So, yesterday and tonight i've taken my laptop and made a lot of testings. First of all, i'll thank forever ... The New York Times ... Why will you tell me ? Because of http://search.cpan.org/~adamk/Aspect-Library-NYTProf-1.00/lib/Aspect/Library... This tool is really amazing. You run perl -d:NYTProf mainpage.pl ; nytprofhtml --open And you get a highly detailled HTML report with the time spent in each line/sub/module to run mainpage.pl. Here is the result : http://depot.biblibre.com/ppoulain/nytprof.ini/ (the .ini is to keep track of the perf before anything optimized) Note : tests done on my laptop, that is not a server. 1st test with master: Profile of mainpage.pl for 1.73s (of 2.18s), executing 155108 statements and 41041 subroutine calls in 249 source files and 64 string evals. I have investigated to find the longest timings & if we can do something. (note All the following improvement have been done incrementally) First of all, C4::Context and C4::Koha are loaded on everypage, so it's worth investigating == C4::Context == * spent 185ms just reading the config file. Ouch ! that's almost 10% of the time !!! (http://depot.biblibre.com/ppoulain/nytprof.ini/C4-Context-pm-13-line.html#23...) I tried to hardcode the hashref instead of XMLin-ing the config file, the results are astonishing : after = Profile of mainpage.pl for 1.59s (of 1.93s), executing 117399 statements and 26939 subroutine calls in 241 source files and 63 string evals. => PROPOSAL = add a YAML version of the XML config file that we could use in // => PROPOSAL 2 = memcache the config file (hint : the config file in koha 2 was not xml !) * C4::Context, sub db_scheme2dbi, the i flag in /mysql/i cost 84ms !!! with /i, the duration is 84.4ms, without, it's 10µs !!! after = Profile of mainpage.pl for 1.55s (of 1.91s), executing 117398 statements and 26938 subroutine calls in 241 source files and 63 string evals. => PROPOSAL = just 'return "mysql"' at the beginning of this sub, as everything else is not working anyway ! == C4::Languages == * C4::Languages::getAllLanguages 43ms => heavily under optimized mysql query = returns all languages from language_subtag_registry and language_descriptions even if only a few localisations are installed. * C4::Languages::_build_languages_arrayref 15ms => same as for getAllLanguages I know there is a patch (from hdl) pending to improve this sub, i haven't tested it yet (http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6000) (hint : this sub appeared in 3.0, was not in 2.2 !) == Other packages == * C4::Koha.pm 15ms for use Business::ISBN, that is used only in 1 sub used only in a few pages (and none if no cover images are enabled unless i'm wrong) * C4::Letters use MIME::Lite spend 26ms, that are useless 90% of the time (why is C4::Letters loaded ! answer = because C4::Reserves is loaded by Items.pm that is loaded by Biblio.pm that is loaded by AuthoritiesMarc.pm that is loaded (uselessly) by mainpage.pl * CGI/Session/Driver/mysql.pm needs 79ms i've tested by switching to driver::File::Serializer (dropped to 21ms). this seems to work, but probably requires more testing than what I did * Date::Manip needs requires 46ms and is useless most of the time. after = Profile of mainpage.pl for 1.31s (of 1.60s), executing 103643 statements and 25380 subroutine calls in 215 source files and 55 string evals. => PROPOSAL = WHERE it's interesting we could replace the use XXXX by a require XXXX just before the lines that need the package. The require is loaded at run tim, so, when unneeded, the package won't be loaded == mainpage.pl == Mainpage.pl load some authorities, we do nothing with it. Commenting the use C4::Authorities and all related code results in: after = Profile of mainpage.pl for 1.27s (of 1.57s), executing 103000 statements and 25194 subroutine calls in 212 source files and 55 string evals. == Packages nested == * I also noticed that Auth.pm loads Members.pm (to retrieve patron informations and display them on the top right). Members.pm loads Reserves that loads Biblio.pm, Circulation.pm and Items.pm All those packages are the biggest to load. I've written a script, that display, for each file in C4 which package is loaded and which sub in the package loaded are used. It shows, for example, that there is a "use Koha.pm;" that is useless in AuthoritiesMarc.pm It also shows which sub are used, and how many times. For example, Circulation.pm calls "use C4::Biblio" and : In Circulation.pm the Biblio.pm, sub GetBiblioItemData is used 1 times In Circulation.pm the Biblio.pm, sub GetBiblioFromItemNumber is used 4 times That will be handy to find what can be cleaned & removed. I've attached the script and the result. If you want to run it yourself, just export PERL5LIB, KOHA_CONF, put the script in $KOHA directory, and run it ! == Other == * packages that uses the biggest amount of CPU are not C4 packages : * utf8_heavy-pl is the most cpu consuming thing : 132ms, I don't know if we can do something * Template::Parser needs 123ms, not sure we can do something * all XML stuff is highly consuming = loading MARC::File::XML requires 47ms in Biblio.pm, XML::Sax::Base require 40ms to be loaded => definetly, the best way to reduce CPU time is by pre-loading things, we already knew that == Next step == * i'll file a bug for the various improvements that can be made easily * i'll continue investigate & report my findings I'm aware it's a 1st step, and the long-term road is to reach data&code persistency. But cleaning what we already have may ease the next big step. And any speed improvement is a good thing, isn't it ! HTH -- Paul POULAIN http://www.biblibre.com Expert en Logiciels Libres pour l'info-doc Tel : (33) 4 91 81 35 08
Hi Paul Good email, just a couple of comments inline. 2011/9/15 Paul Poulain <paul.poulain@biblibre.com>:
(this mail has been sent 10 hours ago, seems it was too large. I send it again without attached docs)
Le 30/03/2011 16:47, Ian Walls a écrit :
Fellow Developers, Hello everybody, Last night, I stayed up late running circ/circulation.pl <http://circulation.pl> through NYTProf, to get an idea where we may be able to optimize circulation for speed. After much frustration (darn session IDs...), I was able to get a report. The results were... not exactly what I expected.
For those who have used or developed on Koha 2.x, you probably have noticed that Koha 2.x was much more slower than Koha 3.x So, yesterday and tonight i've taken my laptop and made a lot of testings.
First of all, i'll thank forever ... The New York Times ... Why will you tell me ? Because of http://search.cpan.org/~adamk/Aspect-Library-NYTProf-1.00/lib/Aspect/Library...
This tool is really amazing. You run perl -d:NYTProf mainpage.pl ; nytprofhtml --open And you get a highly detailled HTML report with the time spent in each line/sub/module to run mainpage.pl. Here is the result : http://depot.biblibre.com/ppoulain/nytprof.ini/ (the .ini is to keep track of the perf before anything optimized)
Note : tests done on my laptop, that is not a server. 1st test with master: Profile of mainpage.pl for 1.73s (of 2.18s), executing 155108 statements and 41041 subroutine calls in 249 source files and 64 string evals.
I have investigated to find the longest timings & if we can do something. (note All the following improvement have been done incrementally)
First of all, C4::Context and C4::Koha are loaded on everypage, so it's worth investigating
== C4::Context == * spent 185ms just reading the config file. Ouch ! that's almost 10% of the time !!! (http://depot.biblibre.com/ppoulain/nytprof.ini/C4-Context-pm-13-line.html#23...) I tried to hardcode the hashref instead of XMLin-ing the config file, the results are astonishing : after = Profile of mainpage.pl for 1.59s (of 1.93s), executing 117399 statements and 26939 subroutine calls in 241 source files and 63 string evals. => PROPOSAL = add a YAML version of the XML config file that we could use in // => PROPOSAL 2 = memcache the config file (hint : the config file in koha 2 was not xml !)
Done already http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6193 Some more testing on this, so I can push it would be much appreciated.
* C4::Context, sub db_scheme2dbi, the i flag in /mysql/i cost 84ms !!! with /i, the duration is 84.4ms, without, it's 10µs !!! after = Profile of mainpage.pl for 1.55s (of 1.91s), executing 117398 statements and 26938 subroutine calls in 241 source files and 63 string evals. => PROPOSAL = just 'return "mysql"' at the beginning of this sub, as everything else is not working anyway !
== C4::Languages == * C4::Languages::getAllLanguages 43ms => heavily under optimized mysql query = returns all languages from language_subtag_registry and language_descriptions even if only a few localisations are installed. * C4::Languages::_build_languages_arrayref 15ms => same as for getAllLanguages I know there is a patch (from hdl) pending to improve this sub, i haven't tested it yet (http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6000) (hint : this sub appeared in 3.0, was not in 2.2 !)
These are cached by memoize_memcached if you are running it, I strongly suggest people do Chris
Hi, I've continued my work in the train, here is a result of my work. I've checked a lot of the packages in C4 and checked for dependancies (sub packages used) With the check_subs.pl script, it was quite easy to find which subs were called and see if it's needed. What I propose next is to file a bug for each package with my findings and my suggestions. Then, write a patch. I also plan to investigate some of the most used pages (like circ/circulation.pl, circ/return.pl, detail.pl, and most opac pages. Note that the check_subs can be also use to check package dependancies for perl scripts as well: on line 39, set "dirToCheck" to the directory you want to check. I've checked circ and opac, and have already discovered some interesting informations. If you have some comment, want to make another suggestion or give a hand, you're welcomed ;-) mainpage.pl loads the following .pm ==== C4::Auth.pm ==== == use Members.pm == only to call GetMemberDetails and fill USER_INFO parameters USER_INFOS are used in a large variety of place. * In a strange way, like in : acqui/acqui-home.pl: $template->{VARS}->{'USER_INFO'}[0]->{'borrowernumber'} ); Q: don't we have another way to retrieve loggedin librarian number ? * in opac-reserve, to display who we place a request for. Again, this could/should be moved to opac-reserve, and we use GetMemberDetails in this sub CONCLUSION = we should be able to get rid of use Members.pm in Auth.pm == use C4::Context.pm == package that can't be avoided and loads nothing specific == use C4::Output.pm == = use C4::Budgets = just to call Budget::GetCurrency in the Output::FormatNumber sub. The Output::FormatNumber sub is used only in a few places, related to C4::Budgets CONCLUTION => move FormatNumber outside from C4::Output into C4::Budgets and we will get rid of this dependancy = use C4::Languages = to call a lot of sub here, no possibility to do something = use C4::Dates = to call format_date, in Date::FormatDate in C4::Output::FormatData sub that is called only in aqbudgetperiod.pl and should easily be removed. it contains only $$data_hashref{$_} = format_date( $$data_hashref{$_} ) for grep{/date/} keys (%$data_hashref); that can be inlined in aqbudgetperiod (and it would be better to use the OOo date tools) = use C4::Templates = needed, it's the package that smoothen the transition between H::T::P and T::T == use C4::Branch == = use C4::Koha = just to call get_infos_of once, we could get rid with this sub by inlining and remove the C4::Koha dependancy == use C4::VirtualShelves == * this package is needed in case of an OPAC query (on every opac page we display the public & private lists). I suspect it's not needed in staff interface. Anyone to confirm/argue ? * We use only GetRecentShelves sub, * we put everything in the session (set_shelves_userenv) & context ($session->param('barshelves', $barshelves);) I think there is no use for this. Anyone to confirm/argue ? * also note that the following line my ($total, $pubshelves) = C4::Context->get_shelves_userenv(); # an anonymous user has no 'barshelves'... (C4::Auth, line 301 and the following) seems strange. = use C4::Circulation = and that's the beginning of circular-painfull-loading-everything. In C4::VirtualShelves, there is no use of anything from C4::Circulation. I've commented the line, made a lot of tests, and saw no problem at all. ==== C4::Koha ==== This loading is useless. I've commented the line and saw no problem. ==== C4::AuthoritiesMarc; ==== This part is useless here, we do nothing with the authorities retrieved. ==== C4::NewsChannels ==== == use C4::Context == nothing to say, needed to access the database == use C4::Dates == nothing to say, needed to format the date the news has been published. <<<< END OF mainpage specifics >>>> Continued to check some of the major .pm ==== Member.pm ==== Member.pm, (that is loaded by Auth.pm at the moment) load the following packages: == use Reserves.pm == Reserves.pm is used only to call my @itemswaiting = C4::Reserves::GetReservesFromBorrowernumber( $patroninformation->{'borrowernumber'},'W' ); in C4::Members::patronflags sub I'm not sure we do anything with the result of GetReservesFromBorrowernumber: C4::Members, line 518 contains : my @itemswaiting = C4::Reserves::GetReservesFromBorrowernumber( $patroninformation->{'borrowernumber'},'W' ); my $nowaiting = scalar @itemswaiting; if ( $nowaiting > 0 ) { my %flaginfo; $flaginfo{'message'} = "Reserved items available"; $flaginfo{'itemlist'} = \@itemswaiting; $flags{'WAITING'} = \%flaginfo; } return ( \%flags ); I've some doubts about the need for this code, i've commented it, placed a hold, make it waiting for pickup, I see everything as usual, I saw no difference. It will be worth a bug entry and a patch to test soon ! I've moved the use C4::Members to a require C4::Member just before the GetReservesFromBorrowernumber call NYTProf before commenting : Profile of mainpage.pl for 1.24s (of 1.54s), executing 102597 statements and 25016 subroutine calls in 212 source files and 55 string evals. NYTProf after commenting : Profile of mainpage.pl for 1.25s (of 1.55s), executing 102489 statements and 24984 subroutine calls in 212 source files and 55 string evals. (the duration is strange. But I was with my laptop on the battery, so that may explain. The number of statements&subroutines are lowered though) == use C4::Overdues.pm == this is loaded only to call checkoverdues in C4::Members::GetMemberDetails The result is stored in the big member detail hash returned : my ( $odues, $itemsoverdue ) = checkoverdues($patroninformation->{'borrowernumber'}); if ( $odues && $odues > 0 ) { my %flaginfo; $flaginfo{'message'} = "Yes"; $flaginfo{'itemlist'} = $itemsoverdue; foreach ( sort { $a->{'date_due'} cmp $b->{'date_due'} } @$itemsoverdue ) { $flaginfo{'itemlisttext'} .= "$_->{'date_due'} $_->{'barcode'} $_->{'title'} \n"; # newline is display layer } $flags{'ODUES'} = \%flaginfo; It's used only in circulation.pl, return.pl and SIP.pm My opinion is that we could have a dedicated sub for that in Circulation.pm (that is always loaded in circulation.pl & return.pl), retrieve the result from here when needed and reduce the size of GetMemberDetails (that would not return everything) == use C4::Biblio.pm == this is loaded just to call C4::Biblio::GetBiblioFromItemNumber in C4::Members::GetMemberAccountRecords to retrieve title and author This can be easily circumvented with a correct LEFT join a few lines before. Instead of SELECT * FROM accountlines WHERE borrowernumber write : SELECT accountlines.*,biblio.title,biblio.author FROM accountlines LEFT JOIN items USING(itemnumber) LEFT JOIN biblio USING(biblionumber) WHERE borrowernumber The use C4::Biblio can be removed ! == use C4::Account.pm == this is loaded to call manualinvoice twice. the manualinvoice is used only when adding or renewing a patron. So it should be efficient to move from a use C4::Account to a require C4::Account where applicable ==== Accounts.pm ==== == use C4::Stats == this package contains only one sub (updatestats) and no dependancies == use C4::Members == loaded just to call C4::Members::GetMember once in the C4::Accounts::returnlost sub. This sub seems totally unused in Koha (dead code) grep -R "returnlost" * shows only the sub header itself. Could be removed. == use C4::Items == loaded to call ModItem twice. The 1st call is in C4::Accounts::returnlost, that can be removed (see below) The 2nd call is in C4::Accounts::chargelostitem, that is rarely used. Switching to require C4::Items should be possible == use C4::Circulation == This loading is similar to C4::Items with C4::Circulation::MarkIssueReturned sub : The 1st call is in C4::Accounts::returnlost, that can be removed (see below) The 2nd call is in C4::Accounts::chargelostitem, that is rarely used. Switching to require C4::Circulation should be possible ==== Acquisition.pm ==== There is a use Debug.pm that is useless in Acquisition.pm There is a use Biblio.pm that is useless in Acquisition.pm use C4::Debug is written twice ! == use C4::Suggestions == the ModSuggestion and GetSuggestionFromBiblionumber are called when an order is recieved. Could switch to require, both statements are close ==== Biblio.pm ==== == use C4::Serials == Serials.pm is loaded to delete subscriptions in C4::Biblio::DelBiblio. 2 subs are used : C4::Serials::GetFullSubscriptionsFromBiblionumber and C4::Serials::DelSubscription both can be moved to require at the right place == use C4::Items == * the check_subs script wrongly report a use of ModItem and DelItem, it's a comment in the code : # duplication or incorrect data - use AddItem() or ModItem() * C4::Biblio uses C4::Items::GetMarcItem twice * once in PrepareItemrecordDisplay => this sub sounds at a wrong place. It could/should be moved to C4::Items itself. PrepareItemrecordDisplay is used by: * acqui/addorderiso2709.pl = already loads C4::Items * acqui/neworderempty.pl = don't load C4::Items for instance * acqui/orderreceive.pl = already loads C4::Items * C4/Serials.pm = need more investigation * serials/serials-edit.pl = already loads C4::Items * once in C4::Biblio::EmbedItemsInMarcBiblio Could we remove this sub ? question still pending. it's used only in: * C4::Biblio::GetMarcBiblio * misc/migration_tools/rebuild_zebra.pl * tools/export.pl ==== Items.pm ==== == use C4::Biblio == Too many things are required, we can't change. The best solution would be to remove C4::Items reference in C4::Biblio GetOrderFromItemnumber == use C4::Reserves == This package is loaded just for C4::Reserves::CheckReserves called in C4::Items::GetItemsInfo The GetItemsInfo stores the result of CheckReserves in a hash entry, count_reserve, that is used only in opac_detail to display the status of a hold. We could remove the reserve_count hash entry and inline C4::Reserves::CheckReserves directly from opac-detail.pl page in opac-detail.pl, instead of if( $itm->{'count_reserves'} eq "Waiting"){ $itm->{'waiting'} = 1; } write : if ( C4::Reserves::CheckReserves(<<parameters>>) eq "Waiting"){ $itm->{'waiting'} = 1; } == use C4::Acquisition == This package is loaded to call * C4::Acquisition::ModOrder = recent addition, when you move an item to another biblio, you move the order if it's relevant. This sub is rarely called and could be switched to require * C4::Acquisition::GetOrderFromItemnumber = same comment, used a few lines before ModOrder ==== Reserves.pm ==== Reserves.pm is really crapy and should probably be rewritten entirely. A grep -R "use C4::Reserves;" *|wc -l says that 33 scripts are using this package. There is a use Search.pm that is useless in Reserves.pm == use C4::Biblio == loaded to call GetBiblioItemData and GetBiblioData: * GetBiblioData is called in AddReserve, to prepare and send a mail depending on the syspref emailLibrarianWhenHoldIsPlaced * GetBiblioItemData is used twice in the same sub C4::Reserves::GetReservesFromBiblionumber. Those informations are retrieved just to get title,author,... and could be added into the previous SQL query : instead of: SELECT biblioitemnumber FROM reserveconstraints WHERE biblionumber = ? AND borrowernumber = ? AND reservedate = ? write: SELECT reserveconstraints.biblionumber,biblio.*,biblioitems.* FROM reserveconstraints LEFT JOIN biblio USING(biblionumber) LEFT JOIN biblioitems USING(biblionumber) WHERE biblionumber = ? AND borrowernumber = ? AND reservedate = ? That should do the job, and the dependancy could be removed. Note: C4::Reserves::GetReservesFromBiblionumber is used in various places, for example opac-reserve.pl or in catalogue/detail.pl, but it seems we don't use all of the result computed in the sub. So maybe we can just clean GetReservesFromBiblionumber. It must be checked better == use C4::Members == loaded to call: * C4::Members::GetMember is called in * C4::Reserves::AddReserve, to prepare and send a mail depending on the syspref emailLibrarianWhenHoldIsPlaced * C4::Reserves::CanItemBeReserved. This call is just to retrieve patron category and branch. The CanItemBeReserved is called only in request & opac-reserve & ILSDI pages. an option would be to send categorycode and branchcode from here (and change the API of the sub) * C4::Reserves::_koha_notify_reserve, to retrieve the Email (GetMember is always called, but the email is then retrieved from it or from GetFirstValidEmailAddress (see below, it's a dirty code...) * C4::Members::GetFirstValidEmailAddress GetFirstValidEmailAddress is called to send emails to patron that places a reserve, I don't see how to remove this dependancy... * C4::Members::GetMemberDetails is called in C4::Reserves::CheckReserves This sub is probably the crappiest in Koha. It mixes many things, it should be entirely rewritten Conclusion for C4::Reserves: I think we should try to isolate as much as possible this package. -- Paul POULAIN http://www.biblibre.com Expert en Logiciels Libres pour l'info-doc Tel : (33) 4 91 81 35 08
Paul, Excellent! I'd been meaning to do a complete workup on the system through NYTProf when I got the time, and it looks like you've beat me too it. Can you post the full reports somewhere online, with some kind of guide as to what scripts are which, which system preferences you had, and what input params you passed? I've got NYTProf on NEKLS test system (with their permission) and find that that's an excellent resource for testing pages under various kinds of load. It helped me fine the root of bug 6801 (which I still need to patch :/ ) Cheers, -Ian On Wed, Sep 14, 2011 at 5:27 PM, Paul Poulain <paul.poulain@biblibre.com>wrote:
Hi,
I've continued my work in the train, here is a result of my work.
I've checked a lot of the packages in C4 and checked for dependancies (sub packages used) With the check_subs.pl script, it was quite easy to find which subs were called and see if it's needed.
What I propose next is to file a bug for each package with my findings and my suggestions. Then, write a patch. I also plan to investigate some of the most used pages (like circ/circulation.pl, circ/return.pl, detail.pl, and most opac pages. Note that the check_subs can be also use to check package dependancies for perl scripts as well: on line 39, set "dirToCheck" to the directory you want to check. I've checked circ and opac, and have already discovered some interesting informations. If you have some comment, want to make another suggestion or give a hand, you're welcomed ;-)
mainpage.pl loads the following .pm ==== C4::Auth.pm ==== == use Members.pm == only to call GetMemberDetails and fill USER_INFO parameters USER_INFOS are used in a large variety of place. * In a strange way, like in : acqui/acqui-home.pl: $template->{VARS}->{'USER_INFO'}[0]->{'borrowernumber'} ); Q: don't we have another way to retrieve loggedin librarian number ? * in opac-reserve, to display who we place a request for. Again, this could/should be moved to opac-reserve, and we use GetMemberDetails in this sub
CONCLUSION = we should be able to get rid of use Members.pm in Auth.pm == use C4::Context.pm == package that can't be avoided and loads nothing specific == use C4::Output.pm == = use C4::Budgets = just to call Budget::GetCurrency in the Output::FormatNumber sub. The Output::FormatNumber sub is used only in a few places, related to C4::Budgets CONCLUTION => move FormatNumber outside from C4::Output into C4::Budgets and we will get rid of this dependancy = use C4::Languages = to call a lot of sub here, no possibility to do something = use C4::Dates = to call format_date, in Date::FormatDate in C4::Output::FormatData sub that is called only in aqbudgetperiod.pl and should easily be removed. it contains only $$data_hashref{$_} = format_date( $$data_hashref{$_} ) for grep{/date/} keys (%$data_hashref); that can be inlined in aqbudgetperiod (and it would be better to use the OOo date tools) = use C4::Templates = needed, it's the package that smoothen the transition between H::T::P and T::T
== use C4::Branch == = use C4::Koha = just to call get_infos_of once, we could get rid with this sub by inlining and remove the C4::Koha dependancy
== use C4::VirtualShelves == * this package is needed in case of an OPAC query (on every opac page we display the public & private lists). I suspect it's not needed in staff interface. Anyone to confirm/argue ? * We use only GetRecentShelves sub, * we put everything in the session (set_shelves_userenv) & context ($session->param('barshelves', $barshelves);) I think there is no use for this. Anyone to confirm/argue ? * also note that the following line my ($total, $pubshelves) = C4::Context->get_shelves_userenv(); # an anonymous user has no 'barshelves'... (C4::Auth, line 301 and the following) seems strange. = use C4::Circulation = and that's the beginning of circular-painfull-loading-everything. In C4::VirtualShelves, there is no use of anything from C4::Circulation. I've commented the line, made a lot of tests, and saw no problem at all.
==== C4::Koha ==== This loading is useless. I've commented the line and saw no problem.
==== C4::AuthoritiesMarc; ==== This part is useless here, we do nothing with the authorities retrieved.
==== C4::NewsChannels ==== == use C4::Context == nothing to say, needed to access the database == use C4::Dates == nothing to say, needed to format the date the news has been published.
<<<< END OF mainpage specifics >>>> Continued to check some of the major .pm
==== Member.pm ==== Member.pm, (that is loaded by Auth.pm at the moment) load the following packages: == use Reserves.pm == Reserves.pm is used only to call my @itemswaiting = C4::Reserves::GetReservesFromBorrowernumber( $patroninformation->{'borrowernumber'},'W' ); in C4::Members::patronflags sub I'm not sure we do anything with the result of GetReservesFromBorrowernumber: C4::Members, line 518 contains : my @itemswaiting = C4::Reserves::GetReservesFromBorrowernumber( $patroninformation->{'borrowernumber'},'W' ); my $nowaiting = scalar @itemswaiting; if ( $nowaiting > 0 ) { my %flaginfo; $flaginfo{'message'} = "Reserved items available"; $flaginfo{'itemlist'} = \@itemswaiting; $flags{'WAITING'} = \%flaginfo; } return ( \%flags ); I've some doubts about the need for this code, i've commented it, placed a hold, make it waiting for pickup, I see everything as usual, I saw no difference. It will be worth a bug entry and a patch to test soon !
I've moved the use C4::Members to a require C4::Member just before the GetReservesFromBorrowernumber call NYTProf before commenting : Profile of mainpage.pl for 1.24s (of 1.54s), executing 102597 statements and 25016 subroutine calls in 212 source files and 55 string evals. NYTProf after commenting : Profile of mainpage.pl for 1.25s (of 1.55s), executing 102489 statements and 24984 subroutine calls in 212 source files and 55 string evals. (the duration is strange. But I was with my laptop on the battery, so that may explain. The number of statements&subroutines are lowered though)
== use C4::Overdues.pm == this is loaded only to call checkoverdues in C4::Members::GetMemberDetails The result is stored in the big member detail hash returned : my ( $odues, $itemsoverdue ) = checkoverdues($patroninformation->{'borrowernumber'}); if ( $odues && $odues > 0 ) { my %flaginfo; $flaginfo{'message'} = "Yes"; $flaginfo{'itemlist'} = $itemsoverdue; foreach ( sort { $a->{'date_due'} cmp $b->{'date_due'} } @$itemsoverdue ) { $flaginfo{'itemlisttext'} .= "$_->{'date_due'} $_->{'barcode'} $_->{'title'} \n"; # newline is display layer } $flags{'ODUES'} = \%flaginfo; It's used only in circulation.pl, return.pl and SIP.pm My opinion is that we could have a dedicated sub for that in Circulation.pm (that is always loaded in circulation.pl & return.pl), retrieve the result from here when needed and reduce the size of GetMemberDetails (that would not return everything)
== use C4::Biblio.pm == this is loaded just to call C4::Biblio::GetBiblioFromItemNumber in C4::Members::GetMemberAccountRecords to retrieve title and author This can be easily circumvented with a correct LEFT join a few lines before. Instead of SELECT * FROM accountlines WHERE borrowernumber write : SELECT accountlines.*,biblio.title,biblio.author FROM accountlines LEFT JOIN items USING(itemnumber) LEFT JOIN biblio USING(biblionumber) WHERE borrowernumber The use C4::Biblio can be removed ! == use C4::Account.pm == this is loaded to call manualinvoice twice. the manualinvoice is used only when adding or renewing a patron. So it should be efficient to move from a use C4::Account to a require C4::Account where applicable
==== Accounts.pm ==== == use C4::Stats == this package contains only one sub (updatestats) and no dependancies == use C4::Members == loaded just to call C4::Members::GetMember once in the C4::Accounts::returnlost sub. This sub seems totally unused in Koha (dead code) grep -R "returnlost" * shows only the sub header itself. Could be removed. == use C4::Items == loaded to call ModItem twice. The 1st call is in C4::Accounts::returnlost, that can be removed (see below) The 2nd call is in C4::Accounts::chargelostitem, that is rarely used. Switching to require C4::Items should be possible == use C4::Circulation == This loading is similar to C4::Items with C4::Circulation::MarkIssueReturned sub : The 1st call is in C4::Accounts::returnlost, that can be removed (see below) The 2nd call is in C4::Accounts::chargelostitem, that is rarely used. Switching to require C4::Circulation should be possible
==== Acquisition.pm ==== There is a use Debug.pm that is useless in Acquisition.pm There is a use Biblio.pm that is useless in Acquisition.pm use C4::Debug is written twice ! == use C4::Suggestions == the ModSuggestion and GetSuggestionFromBiblionumber are called when an order is recieved. Could switch to require, both statements are close
==== Biblio.pm ==== == use C4::Serials == Serials.pm is loaded to delete subscriptions in C4::Biblio::DelBiblio. 2 subs are used : C4::Serials::GetFullSubscriptionsFromBiblionumber and C4::Serials::DelSubscription both can be moved to require at the right place == use C4::Items == * the check_subs script wrongly report a use of ModItem and DelItem, it's a comment in the code : # duplication or incorrect data - use AddItem() or ModItem() * C4::Biblio uses C4::Items::GetMarcItem twice * once in PrepareItemrecordDisplay => this sub sounds at a wrong place. It could/should be moved to C4::Items itself. PrepareItemrecordDisplay is used by: * acqui/addorderiso2709.pl = already loads C4::Items * acqui/neworderempty.pl = don't load C4::Items for instance * acqui/orderreceive.pl = already loads C4::Items * C4/Serials.pm = need more investigation * serials/serials-edit.pl = already loads C4::Items * once in C4::Biblio::EmbedItemsInMarcBiblio Could we remove this sub ? question still pending. it's used only in: * C4::Biblio::GetMarcBiblio * misc/migration_tools/rebuild_zebra.pl * tools/export.pl
==== Items.pm ==== == use C4::Biblio == Too many things are required, we can't change. The best solution would be to remove C4::Items reference in C4::Biblio GetOrderFromItemnumber == use C4::Reserves == This package is loaded just for C4::Reserves::CheckReserves called in C4::Items::GetItemsInfo The GetItemsInfo stores the result of CheckReserves in a hash entry, count_reserve, that is used only in opac_detail to display the status of a hold. We could remove the reserve_count hash entry and inline C4::Reserves::CheckReserves directly from opac-detail.pl page in opac-detail.pl, instead of if( $itm->{'count_reserves'} eq "Waiting"){ $itm->{'waiting'} = 1; } write : if ( C4::Reserves::CheckReserves(<<parameters>>) eq "Waiting"){ $itm->{'waiting'} = 1; }
== use C4::Acquisition == This package is loaded to call * C4::Acquisition::ModOrder = recent addition, when you move an item to another biblio, you move the order if it's relevant. This sub is rarely called and could be switched to require * C4::Acquisition::GetOrderFromItemnumber = same comment, used a few lines before ModOrder
==== Reserves.pm ==== Reserves.pm is really crapy and should probably be rewritten entirely. A grep -R "use C4::Reserves;" *|wc -l says that 33 scripts are using this package.
There is a use Search.pm that is useless in Reserves.pm == use C4::Biblio == loaded to call GetBiblioItemData and GetBiblioData: * GetBiblioData is called in AddReserve, to prepare and send a mail depending on the syspref emailLibrarianWhenHoldIsPlaced * GetBiblioItemData is used twice in the same sub C4::Reserves::GetReservesFromBiblionumber. Those informations are retrieved just to get title,author,... and could be added into the previous SQL query : instead of: SELECT biblioitemnumber FROM reserveconstraints WHERE biblionumber = ? AND borrowernumber = ? AND reservedate = ? write: SELECT reserveconstraints.biblionumber,biblio.*,biblioitems.* FROM reserveconstraints LEFT JOIN biblio USING(biblionumber) LEFT JOIN biblioitems USING(biblionumber) WHERE biblionumber = ? AND borrowernumber = ? AND reservedate = ? That should do the job, and the dependancy could be removed. Note: C4::Reserves::GetReservesFromBiblionumber is used in various places, for example opac-reserve.pl or in catalogue/detail.pl, but it seems we don't use all of the result computed in the sub. So maybe we can just clean GetReservesFromBiblionumber. It must be checked better
== use C4::Members == loaded to call: * C4::Members::GetMember is called in * C4::Reserves::AddReserve, to prepare and send a mail depending on the syspref emailLibrarianWhenHoldIsPlaced * C4::Reserves::CanItemBeReserved. This call is just to retrieve patron category and branch. The CanItemBeReserved is called only in request & opac-reserve & ILSDI pages. an option would be to send categorycode and branchcode from here (and change the API of the sub) * C4::Reserves::_koha_notify_reserve, to retrieve the Email (GetMember is always called, but the email is then retrieved from it or from GetFirstValidEmailAddress (see below, it's a dirty code...) * C4::Members::GetFirstValidEmailAddress GetFirstValidEmailAddress is called to send emails to patron that places a reserve, I don't see how to remove this dependancy... * C4::Members::GetMemberDetails is called in C4::Reserves::CheckReserves This sub is probably the crappiest in Koha. It mixes many things, it should be entirely rewritten
Conclusion for C4::Reserves: I think we should try to isolate as much as possible this package.
-- Paul POULAIN http://www.biblibre.com Expert en Logiciels Libres pour l'info-doc Tel : (33) 4 91 81 35 08
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Ian Walls Lead Development Specialist ByWater Solutions Phone # (888) 900-8944 http://bywatersolutions.com ian.walls@bywatersolutions.com Twitter: @sekjal
Paul,
Excellent! I'd been meaning to do a complete workup on the system through NYTProf when I got the time, and it looks like you've beat me too it. Can you post the full reports somewhere online, I don't understand what you mean by "full reports" with some kind of guide as to what scripts are which, which system preferences you had, and what input params you passed? well, all this analysis was just based on mainpage.pl run, and investigating C4 modules. I worked on both de-nesting and perf improving. My first tests were mostly done to test performances. It appears that
Le 15/09/2011 13:35, Ian Walls a écrit : the biggest goal now is to lower the package loading cost, thus I moved from perfs to de-nesting. I think de-nesting will greatly help moving forward on perf improving. Plus it will be a good help to reach persistency. Plus it's always better to have a clean code, of course. -- Paul POULAIN http://www.biblibre.com Expert en Logiciels Libres pour l'info-doc Tel : (33) 4 91 81 35 08
Hello world! For ppl looking at bugzilla, you'll already have seen that the bug 6875 has been open to keep track of all my work. I spent 30+ hours on this topic, and 17 patches are attached to this bug, that now require sign-off The final gain is interesting (see http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6875#c20) . My gain evaluation are incomplete, and i'm sure they'll be very important on some pages, and quite limited on some. The packages for circulation & reserves are heavily nested, it's almost impossible to do better without restructuring completly C4, and that will be another goal. In the meantime, if someone feel brave enough to give a try to each patch, the rule is easy to test for signoff = those patches strictly changes nothing in Koha behaviour, it's just internal stuff... For some of them, it will be easy, for a few of them it will require a lot of testing. Chris C. = can you run all automated tests easily or tell me how to do it ? All = i've added this bug as an ENH, but I was hesitating with "Critical". What do you think ? is it just an ENH or a bugfix ? -- Paul POULAIN http://www.biblibre.com Expert en Logiciels Libres pour l'info-doc Tel : (33) 4 91 81 35 08
participants (11)
-
Chris Cormack -
Chris Cormack -
Chris Nighswonger -
Clay Fouts -
Colin Campbell -
Galen Charlton -
Henri-Damien LAURENT -
Ian Walls -
LAURENT Henri-Damien -
Paul Poulain -
Robin Sheat