Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize lazy loading in DatabaseMap #1972

Conversation

alfredbez
Copy link

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.

@alfredbez alfredbez force-pushed the feature/optimize-lazy-loading-in-DatabaseMap branch from ee1406b to 1bfbd35 Compare July 11, 2023 11:51
@mringler
Copy link
Contributor

mringler commented Jul 11, 2023

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
Copy link
Author

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.

@alfredbez alfredbez closed this Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants