search

propelorm / Propel2

Propel2 is an open-source high-performance Object-Relational Mapping (ORM) for modern PHP
http://propelorm.org/
MIT License
1.26k stars 399 forks source link

Optimize lazy loading in DatabaseMap #1972

Closed alfredbez closed 1 year ago

alfredbez commented 1 year ago

This will reduce the overhead of the initialization nearly to zero. The registration of the table maps just stores the mapping information as they're passed into an array (as strings) and loads the classes only when the table is actually requested.

We measured this patch on one of our API endpoints with a fairly low TTFB (<50ms) and we could measure that registerTableMapClasses took 12ms before this patch. With this patch applied registerTableMapClasses does not show up in the profiling result anymore.

mringler commented 1 year ago

Hmm, that's a great change, but I am not sure if it is necessary anymore.

The method you are changing, DatabaseMap::registerTableMapClasses(), is only used in StandardServiceContainer::initDatabaseMaps(), which is the old way of loading the TableMaps. It is still used in the QuickBuilder, a utility class mostly used in testing, and in old configurations. If you have the current version of Propel installed and have updated your configuration or rebuild the models since last November, DatabaseMap::initDatabaseMapFromDumps() is used, which should have the same effect as this patch (basically, the whole DatabaseMap is loaded from a dump instead of registering every TableMap individually, and the TableMap classes are only loaded when a table is accessed, see #1910).

So you shouldn't actually see a performance gain with these changes, simply because the methods should not be used anymore (and because the current version performs at least the same). Can you check if and why DatabaseMap::registerTableMapClasses() is actually used when loading Propel in your instance? Usually, the method is called in the loadDatabase.php file:

<?php
$serviceContainer = \Propel\Runtime\Propel::getServiceContainer();
$serviceContainer->initDatabaseMapFromDumps(array (      // <------------- 
...

Or am I missing something here?

alfredbez commented 1 year ago

Thanks for the info, I'll have a look into that. In fact, we are using a fork from spryker in our project, which does not have the improvements you mentioned.

On a side note, I realized, that the tests are also failing on the master branch with the same failure:

I'll close the PR for now, feel free to ping me if you still think this might be relevant or needed.