codecov-commenter
commented
1 year ago Codecov Report
Base: 87.64% // Head: 87.60% // Decreases project coverage by -0.03% :warning:
Coverage data is based on head (
1ae1e1c) compared to base (f9b9e12). Patch coverage: 78.57% of modified lines in pull request are covered.
Additional details and impacted files
```diff @@ Coverage Diff @@ ## master #1910 +/- ## ============================================ - Coverage 87.64% 87.60% -0.04% - Complexity 7829 7839 +10 ============================================ Files 227 227 Lines 21183 21193 +10 ============================================ + Hits 18565 18567 +2 - Misses 2618 2626 +8 ``` | Flag | Coverage Δ | | |---|---|---| | 5-max | `87.60% <78.57%> (-0.04%)` | :arrow_down: | | 7.4 | `87.60% <78.57%> (-0.04%)` | :arrow_down: | | agnostic | `66.97% <19.64%> (-0.07%)` | :arrow_down: | | mysql | `68.80% <67.27%> (-0.02%)` | :arrow_down: | | pgsql | `68.81% <67.27%> (-0.03%)` | :arrow_down: | | sqlite | `66.65% <72.72%> (-0.02%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/propelorm/Propel2/pull/1910?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm) | Coverage Δ | | |---|---|---| | [src/Propel/Generator/Manager/AbstractManager.php](https://codecov.io/gh/propelorm/Propel2/pull/1910/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm#diff-c3JjL1Byb3BlbC9HZW5lcmF0b3IvTWFuYWdlci9BYnN0cmFjdE1hbmFnZXIucGhw) | `77.20% <0.00%> (ø)` | | | [src/Propel/Generator/Model/Table.php](https://codecov.io/gh/propelorm/Propel2/pull/1910/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm#diff-c3JjL1Byb3BlbC9HZW5lcmF0b3IvTW9kZWwvVGFibGUucGhw) | `91.41% <ø> (ø)` | | | [src/Propel/Generator/Util/QuickBuilder.php](https://codecov.io/gh/propelorm/Propel2/pull/1910/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm#diff-c3JjL1Byb3BlbC9HZW5lcmF0b3IvVXRpbC9RdWlja0J1aWxkZXIucGhw) | `84.95% <ø> (ø)` | | | [src/Propel/Generator/Util/VfsTrait.php](https://codecov.io/gh/propelorm/Propel2/pull/1910/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm#diff-c3JjL1Byb3BlbC9HZW5lcmF0b3IvVXRpbC9WZnNUcmFpdC5waHA=) | `100.00% <ø> (ø)` | | | [...ime/ServiceContainer/ServiceContainerInterface.php](https://codecov.io/gh/propelorm/Propel2/pull/1910/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm#diff-c3JjL1Byb3BlbC9SdW50aW1lL1NlcnZpY2VDb250YWluZXIvU2VydmljZUNvbnRhaW5lckludGVyZmFjZS5waHA=) | `0.00% <ø> (ø)` | | | [...time/ServiceContainer/StandardServiceContainer.php](https://codecov.io/gh/propelorm/Propel2/pull/1910/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm#diff-c3JjL1Byb3BlbC9SdW50aW1lL1NlcnZpY2VDb250YWluZXIvU3RhbmRhcmRTZXJ2aWNlQ29udGFpbmVyLnBocA==) | `83.66% <0.00%> (-3.33%)` | :arrow_down: | | [src/Propel/Runtime/Util/Profiler.php](https://codecov.io/gh/propelorm/Propel2/pull/1910/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm#diff-c3JjL1Byb3BlbC9SdW50aW1lL1V0aWwvUHJvZmlsZXIucGhw) | `83.90% <ø> (ø)` | | | [...nerator/Builder/Om/TableMapLoaderScriptBuilder.php](https://codecov.io/gh/propelorm/Propel2/pull/1910/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm#diff-c3JjL1Byb3BlbC9HZW5lcmF0b3IvQnVpbGRlci9PbS9UYWJsZU1hcExvYWRlclNjcmlwdEJ1aWxkZXIucGhw) | `95.12% <93.75%> (-4.88%)` | :arrow_down: | | [...rc/Propel/Generator/Reverse/SqliteSchemaParser.php](https://codecov.io/gh/propelorm/Propel2/pull/1910/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm#diff-c3JjL1Byb3BlbC9HZW5lcmF0b3IvUmV2ZXJzZS9TcWxpdGVTY2hlbWFQYXJzZXIucGhw) | `89.55% <100.00%> (ø)` | | | [...rc/Propel/Runtime/Connection/ConnectionFactory.php](https://codecov.io/gh/propelorm/Propel2/pull/1910/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm#diff-c3JjL1Byb3BlbC9SdW50aW1lL0Nvbm5lY3Rpb24vQ29ubmVjdGlvbkZhY3RvcnkucGhw) | `77.77% <100.00%> (ø)` | | | ... and [13 more](https://codecov.io/gh/propelorm/Propel2/pull/1910/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=propelorm):umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
profuel
dereuromark
mringler
This MR changes the way tables are registered during initialization to improve performance. Instead of building the internal table lookup structure by accessing static class properties of the table classes, the table lookup is loaded directly from the init file.
Background
During initialization, available database tables are registered in Propel. Currently this is done through the qualified class name (i.e.
\Propel\Tests\BookstoreSchemas\Map\BookTableMap), which is handed to aDatabaseMap, which then adds it to its table lookup maps. There are two kinds of lookup, access by name (schema and table name, i.e.bookstore_schemas.book) or by PHP name (either set in schema.xml or generated from name, i.e.\Book). Currently, the lookup maps are created by accessing static properties of the supplied class. It was reported that this leads to a measurable slowdown when working with lots of tables (300+), see here.Proposed Solution
Instead of handing over only the table class name to the DatabaseMap, we can also hand over the name and PHP name, making the static access obsolete. Even simpler, instead of reading the raw table data from the init file (usually
loadDatabase.php) and then building up the lookup structure every time, we can just write and then retrieve that lookup structure from the init file, and simply load it into the DatabaseMap as it is.1909 does something similar, and it reportedly leads to a speed up.
Changes
DatabaseMapclass gets a method to dump its lookup maps and a method to load them again from dump.TableMapLoaderScriptBuilderclass (which generates theloadDatabase.phpfile) does not just write an array of table class names, but registers the tables in a local DatabaseMap, and then writes a dump to the init file.StandardServiceContainer).The changes do not impact BC, even after the update, an old init file can still be used.
Result
On my machine, with PHP 8.1 and a project with 40 tables, initialization through static access takes around 0.8 ms (+- 0.3ms), while initialization through dumps takes around 0.25 ms (+- 0.1ms). I am actually surprised how clearly and distinct the difference between the two approaches is. I guess it comes from the time it takes PHP to load the classes, but there is also the repeated calls to a subroutine and writing to the lookup arrays individually. I have tested it through the php dev server, not on an actual webserver, which might give different results. Of course we are talking about less than a thousand of a second, so this will not make a difference to most users. But it won't hurt either.
I don't have a project with more tables available, but maybe @profuel can supply some measurements for a larger set of tables? I would assume that the static approach grows linear with the number of tables, as each new table leads to another class being loaded, where the dump approach stays mostly the same, as adding another new table just means to read two more lines from a file.