[Bug 35250] New: Eliminate circular dependencies caused by C4::Context
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Bug ID: 35250 Summary: Eliminate circular dependencies caused by C4::Context Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: philippe.blouin@inlibro.com QA Contact: testopia@bugs.koha-community.org Any script using C4::Context is bound to have a huge memory footprint because of the dependencies. And even script that would do without C4::Context (somehow) are bound for the same as soon as they include one Koha:: module, because those will use C4::Context. A graph from v23.05 will summarize it better than any explanation: BackgroundJobs | v BackgroundJob (Net::Stomp : 5M) | | | v | Koha::Object (Mojo::JSON : 16M) v C4::Context (DateTime::TimeZone : 7M) | | (ZOOM : 8M) v | Koha::Config::Syspref | | v | C4::Log (calling back on C4::Context) | | v | Koha::ActionLogs | | v | Koha::ActionLog (calling back on C4::Context) | | v | Koha::Object | | v Koha::Caches (calling back on C4::Context) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28410 Assignee|koha-bugs@lists.koha-commun |philippe.blouin@inlibro.com |ity.org | CC| |philippe.blouin@inlibro.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 --- Comment #1 from Blou <philippe.blouin@inlibro.com> --- This is a branch from Bug 28410 and Bug 28411, for earlier exchanges. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 --- Comment #2 from Blou <philippe.blouin@inlibro.com> --- Created attachment 158361 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158361&action=edit Bug 35250: Use lazy loading to reduce memory footprint of C4/Context.pm This simply removes some 'use' declarations at the top of the file to instead use 'require' later on. The effect can be validated with a simple one-liner (provided KOHA_CONF and PERL5LIB being set) perl -e "use C4::Context; while(1){sleep(1);}" And monitoring the memoring usage in tools like 'top' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 --- Comment #3 from Blou <philippe.blouin@inlibro.com> --- Created attachment 158362 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158362&action=edit Bug 35250: remove usages of Koha::Config::Syspref(s) from C4::Context Use straigth SQL queries to access systempreferences instead of relying on the ORM, thus removing all direct and indirect dependencies to Koha::Object To remove all dependencies between C4::Context and the db object model, we move the calls to set_preference and delete_prefence directly to the class object Koha::Config::SysPrefs C4::Context::preference remains but uses straight SQL C4::Context::set_preference moves to Koha::Config::SysPrefs::set_preference C4::Context::delete_preference moves to Koha::Config::SysPrefs::delete_preference -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 --- Comment #4 from Blou <philippe.blouin@inlibro.com> --- Created attachment 158363 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158363&action=edit Bug 35250: modify scripts calling C4::Context::set_preference All the scripts using the call C4::Context->set_preference are edited to now use Koha::Config::SysPrefs->set_preference instead of C4::Context->set_preference() One call to delete_preference moved as well -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 --- Comment #5 from Blou <philippe.blouin@inlibro.com> --- Created attachment 158364 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158364&action=edit Bug 35250: modify t/db_dependent tests to use Koha::Config::SysPrefs' set_preference instead of C4::Context Some work left to be done, including missing use at top of files -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com Blocks| |28411 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28411 [Bug 28411] Investigate memory footprint of the different dependencies -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Salah Ghedda <salah.ghedda@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158361|0 |1 is obsolete| | --- Comment #6 from Salah Ghedda <salah.ghedda@inlibro.com> --- Created attachment 159735 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159735&action=edit Bug 35250: Use lazy loading to reduce memory footprint of C4/Context.pm This simply removes some 'use' declarations at the top of the file to instead use 'require' later on. The effect can be validated with a simple one-liner (provided KOHA_CONF and PERL5LIB being set) perl -e "use C4::Context; while(1){sleep(1);}" And monitoring the memoring usage in tools like 'top' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Salah Ghedda <salah.ghedda@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158362|0 |1 is obsolete| | --- Comment #7 from Salah Ghedda <salah.ghedda@inlibro.com> --- Created attachment 159736 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159736&action=edit Bug 35250: remove usages of Koha::Config::Syspref(s) from C4::Context Use straigth SQL queries to access systempreferences instead of relying on the ORM, thus removing all direct and indirect dependencies to Koha::Object To remove all dependencies between C4::Context and the db object model, we move the calls to set_preference and delete_prefence directly to the class object Koha::Config::SysPrefs C4::Context::preference remains but uses straight SQL C4::Context::set_preference moves to Koha::Config::SysPrefs::set_preference C4::Context::delete_preference moves to Koha::Config::SysPrefs::delete_preference -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Salah Ghedda <salah.ghedda@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158363|0 |1 is obsolete| | --- Comment #8 from Salah Ghedda <salah.ghedda@inlibro.com> --- Created attachment 159737 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159737&action=edit Bug 35250: modify scripts calling C4::Context::set_preference All the scripts using the call C4::Context->set_preference are edited to now use Koha::Config::SysPrefs->set_preference instead of C4::Context->set_preference() One call to delete_preference moved as well -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Salah Ghedda <salah.ghedda@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158364|0 |1 is obsolete| | --- Comment #9 from Salah Ghedda <salah.ghedda@inlibro.com> --- Created attachment 159738 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159738&action=edit Bug 35250: modify t/db_dependent tests to use Koha::Config::SysPrefs' set_preference instead of C4::Context Some work left to be done, including missing use at top of files -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Salah Ghedda <salah.ghedda@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |salah.ghedda@inlibro.com --- Comment #10 from Salah Ghedda <salah.ghedda@inlibro.com> --- Rebased -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159735|0 |1 is obsolete| | --- Comment #11 from Blou <philippe.blouin@inlibro.com> --- Created attachment 163733 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163733&action=edit Bug 35250: Use lazy loading to reduce memory footprint of C4/Context.pm This simply removes some 'use' declarations at the top of the file to instead use 'require' later on. The effect can be validated with a simple one-liner (provided KOHA_CONF and PERL5LIB being set) perl -e "use C4::Context; while(1){sleep(1);}" And monitoring the memoring usage in tools like 'top' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159736|0 |1 is obsolete| | --- Comment #12 from Blou <philippe.blouin@inlibro.com> --- Created attachment 163734 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163734&action=edit Bug 35250: remove usages of Koha::Config::Syspref(s) from C4::Context Use straigth SQL queries to access systempreferences instead of relying on the ORM, thus removing all direct and indirect dependencies to Koha::Object To remove all dependencies between C4::Context and the db object model, we move the calls to set_preference and delete_prefence directly to the class object Koha::Config::SysPrefs C4::Context::preference remains but uses straight SQL C4::Context::set_preference moves to Koha::Config::SysPrefs::set_preference C4::Context::delete_preference moves to Koha::Config::SysPrefs::delete_preference -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159737|0 |1 is obsolete| | --- Comment #13 from Blou <philippe.blouin@inlibro.com> --- Created attachment 163735 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163735&action=edit Bug 35250: modify scripts calling C4::Context::set_preference All the scripts using the call C4::Context->set_preference are edited to now use Koha::Config::SysPrefs->set_preference instead of C4::Context->set_preference() One call to delete_preference moved as well -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159738|0 |1 is obsolete| | --- Comment #14 from Blou <philippe.blouin@inlibro.com> --- Created attachment 163736 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163736&action=edit Bug 35250: modify t/db_dependent tests to use Koha::Config::SysPrefs' set_preference instead of C4::Context Some work left to be done, including missing use at top of files -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 --- Comment #15 from Jonathan Druart <jonathan.druart@gmail.com> --- Replacing use with require is not recommended. IMO the correct fix here would be to move sysprefs code outside of C4::Context. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 --- Comment #16 from Jonathan Druart <jonathan.druart@gmail.com> --- Sorry, it's what you are actually doing in the other patches. I think that Koha::Config::Syspref should not use C4::Context, so that you could fetch sysprefs without the whole memory stack. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 --- Comment #17 from Blou <philippe.blouin@inlibro.com> --- If that's allowed, I'll explore that. I just got back to it (thus the recent rebase) and applied them on our 23.05s to validate nothing unexpected happens (and I need some RES memory back on our servers :-) ) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 --- Comment #18 from Blou <philippe.blouin@inlibro.com> --- It is very hard to decouple those fully. Config::SysPrefs uses $C4::Context::use_syspref_cache, which is in C4::Context because the sub preference() is still there and might use the cache. Of course, all this could/should be moved to Koha/Config/SysPrefs, but we're talking hundred of calls to change from C4::Context->preference to Koha::Config::SysPrefs->preference(), and that would be hell to rebase repetitively. Right now (locally), I get a 50M footprint to simple use of Koha::Config::SysPrefs, and 21M for C4::Context. I don't think SysPrefs can live without C4/Context because how would it find its DB connection? Somewhere, SysPrefs needs Context. But at least, Context doesn't need SysPrefs and that would make a big difference to a lot of scripts if we could decouple Context completely from the db objects. If I understand right. In short, the only thing I see I could do "more" is moving the preference subs to SysPrefs, but that seems like a task to automate just before a Pushed to Master, not something that can be rebased for months. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=41890 --- Comment #19 from Jonathan Druart <jonathan.druart@gmail.com> --- I have submitted several patches under bug 41890. Please have a look if you are still interested in this topic. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35250 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |DUPLICATE --- Comment #20 from Jonathan Druart <jonathan.druart@gmail.com> --- *** This bug has been marked as a duplicate of bug 42662 *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org