Bug 15350 - DBIx::Class Startup speed
Summary: DBIx::Class Startup speed
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 15342
  Show dependency treegraph
 
Reported: 2015-12-10 11:55 UTC by Jonathan Druart
Modified: 2016-04-21 06:44 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
DBIX call stacks (90.67 KB, image/png)
2015-12-11 11:50 UTC, Frédéric Demians
Details
Bug 15350: Cache loaded DBIx::Class schema classes (2.44 KB, patch)
2016-03-01 20:52 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15350: Move the import of Koha::Schema to a basic `use` (1.44 KB, patch)
2016-03-03 21:34 UTC, Jesse Weaver
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2015-12-10 11:55:44 UTC
(This follows 15341 comment 5)

From https://metacpan.org/pod/distribution/DBIx-Class/lib/DBIx/Class/Manual/Cookbook.pod#STARTUP-SPEED
There are some tips to follow to reduce the DBIx::Class startup delays.

===
1 - Statically Define Your Schema

If you are using DBIx::Class::Schema::Loader to build the classes dynamically based on the database schema then there will be a significant startup delay.

For production use a statically defined schema (which can be generated using DBIx::Class::Schema::Loader to dump the database schema once - see make_schema_at and dump_directory for more details on creating static schemas from a database).

2 - Move Common Startup into a Base Class

Typically DBIx::Class result classes start off with

  use base qw/DBIx::Class::Core/;
  __PACKAGE__->load_components(qw/InflateColumn::DateTime/);

If this preamble is moved into a common base class:-

  package MyDBICbase;
 
  use base qw/DBIx::Class::Core/;
  __PACKAGE__->load_components(qw/InflateColumn::DateTime/);
  1;

and each result class then uses this as a base:-

  use base qw/MyDBICbase/;

then the load_components is only performed once, which can result in a considerable startup speedup for schemas with many classes.

3 - Explicitly List Schema Result Classes

The schema class will normally contain

  __PACKAGE__->load_classes();

to load the result classes. This will use Module::Find to find and load the appropriate modules. Explicitly defining the classes you wish to load will remove the overhead of Module::Find and the related directory operations:

  __PACKAGE__->load_classes(qw/ CD Artist Track /);

If you are instead using the load_namespaces syntax to load the appropriate classes there is not a direct alternative avoiding Module::Find.

4 - MEMORY USAGE

Cached statements

DBIx::Class normally caches all statements with prepare_cached(). This is normally a good idea, but if too many statements are cached, the database may use too much memory and may eventually run out and fail entirely.
Comment 1 Jonathan Druart 2015-12-10 12:00:11 UTC
1 - Already done

2 - We don't load any components at the moment

3 - In Koha::Schema, I have tried to replace
  __PACKAGE__->load_namespaces;
with
  my $r = [ qw( Accountline Accountoffset ... Zebraqueue ) ];
  __PACKAGE__->load_classes({ 'Koha::Schema::Result' => $r});

I have not noticed any gain using the following script
  use Koha::Database;
  Koha::Database->new()->schema()->resultset('Discharge');

Always between 0.85 and 0.90 second

4 - Something to try?
Comment 2 Frédéric Demians 2015-12-11 11:50:41 UTC
Created attachment 45606 [details]
DBIX call stacks

This graph show the time spent in DBIx::Class::Schema::load_namespace by a script which does nothing but creating a connection to Koha DB. It take half a second on recent CPU. The question for me is whether the schema creation, which implies creating thousand of classes, is redone each time a Koha page is loaded, or if it's cached under Plack.
Comment 3 Jesse Weaver 2016-03-01 20:52:33 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-03-02 08:36:14 UTC
(In reply to Jesse Weaver from comment #3)
> Created attachment 48519 [details] [review] [review]
> Bug 15350: Cache loaded DBIx::Class schema classes
> 
> This separates the Koha::Schema creation into two stages:
> 
>  1) The loading of the schemas themselves, which is done upon import of
>     Koha::Database. This requires moving the import of Koha::Schema from
>     an on-demand `require` to just a `use`; most everything in Koha uses
>     C4::Context, which will indirectly end up calling
>     Koha::Database->schema->new anyway, so this was no longer saving
>     anything.

Yes, I totally agree. This was previously done when our Plack implementation was not considered stable and we did not want to load the schema everywhere. Now we want.

>  2) The actual database connection, which is done by cloning the schema
>     created above and adding a database connection.

I am not sure about this change, the Koha::Schema->connect will load all the classes (see Koha::Schema:  12 __PACKAGE__->load_namespaces;).

> Note that this saves time only when a Plack-based Koha needs to create
> a database connection, which is usually only the case for a new worker.
> 
> Unscientific timings before patch:
>   * First load of koha2marclinks.pl on a new worker: 0.70 seconds
>   * Each load after: 0.17 seconds
> 
> After patch:
>   * First load: 0.31 seconds
>   * Each load after: 0.17 seconds

Are you sure it's brought by the ->clone call or just the move from require to use? :)
Comment 5 Jesse Weaver 2016-03-03 21:34:28 UTC
Created attachment 48653 [details] [review]
Bug 15350: Move the import of Koha::Schema to a basic `use`

This moves the import of Koha::Schema from an on-demand `require` to
just a `use`; most everything in Koha uses C4::Context, which will
indirectly end up calling Koha::Database->schema->new anyway, so this
was no longer saving anything.

Note that this saves time only when a Plack-based Koha needs to create
a database connection, which is usually only the case for a new worker.

Unscientific timings before patch:
  * First load of koha2marclinks.pl on a new worker: 0.70 seconds
  * Each load after: 0.17 seconds

After patch:
  * First load: 0.31 seconds
  * Each load after: 0.17 seconds

Jonathan, you seem to be right. NYTProf was making me think we needed to do both, but timings suggest otherwise.
Comment 6 Jonathan Druart 2016-03-04 09:01:52 UTC
Jesse,
I finally don't think this patch is worth the gain: for the scripts not using Plack (misc/*), they will load of the schema even if it's not needed.
Comment 7 Tomás Cohen Arazi 2016-03-07 12:54:52 UTC
(In reply to Jonathan Druart from comment #6)
> Jesse,
> I finally don't think this patch is worth the gain: for the scripts not
> using Plack (misc/*), they will load of the schema even if it's not needed.

Maybe it is time to think Plack-wise.