View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0034526 | mantisbt | performance | public | 2024-08-02 07:39 | 2024-09-27 01:21 |
Reporter | aaribaud-fgs | Assigned To | community | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Target Version | 2.26.3 | Fixed in Version | 2.26.3 | ||
Summary | 0034526: Bad performance when editing a project having a lot of subprojects | ||||
Description | In the case where this bug was found, there is a parent project with about 1700 sub-projects. | ||||
Steps To Reproduce | Create a (parent) project. | ||||
Additional Information | After analyzing the database queries, it appears that the issue is with the way project_hierarchy_cache() works and is used in the project edit page. The way project_hierarchy_cache() works is:
The way project_hierarchy_cache() is used in the project edit page is : for each sub-project, one call for with $p_show_disabled false, and one with $p_show_disabled true. This negates caching entirely. | ||||
Tags | No tags attached. | ||||
Attached is a possible patch, tested locally on 2.26.2. It lowers project edit load time for 1700 sub-projects from about 20 seconds to about 350 ms. 0001-Fix-project_hierarchy_cache-to-cache-both-full-and-e.patch (3,922 bytes)
From 6eb413e76d01bb8dcd48a2a809980a1a59b16e9f Mon Sep 17 00:00:00 2001 From: Albert ARIBAUD <albert.aribaud@f-g.fr> Date: Fri, 2 Aug 2024 13:40:00 +0200 Subject: [PATCH] Fix project_hierarchy_cache() to cache both full and enabled-only hierarchies The way project_hierarchy_cache() works is: it caches the hierarchy of either all projects or only the enabled projects, based on the value of $p_show_disabled. consecutive calls with the same value for $p_show_disabled work as expected, the first one building the cache, the subsequent ones reusing it. consecutive calls with different values for $p_show_disabled induce a cache rebuild. The way project_hierarchy_cache() is used in the project edit page is : for each sub-project, one call for with $p_show_disabled false, and one with $p_show_disabled true. This negates caching entirely. This patch manages two caches, one for the full hierarchy, and one for enabled projects only. With this patch, the edit page load page for a project with about 1700 sub- projects goes down from about 20 seconds to about 350 ms. diff --git a/core/project_hierarchy_api.php b/core/project_hierarchy_api.php index 2d1161794..70a4431b4 100644 --- a/core/project_hierarchy_api.php +++ b/core/project_hierarchy_api.php @@ -31,9 +31,15 @@ require_api( 'constant_inc.php' ); require_api( 'database_api.php' ); +# We may need to build caches for either all projects or enabled projects or both +$g_cache_project_hierarchy_enabled = null; +$g_cache_project_hierarchy_all = null; +$g_cache_project_inheritance_enabled = null; +$g_cache_show_disabled_all = null; +# To make it simpler for callers of project_hierarchy_cache(), we provide +# these two aliases for the correct pair of caches above $g_cache_project_hierarchy = null; $g_cache_project_inheritance = null; -$g_cache_show_disabled = null; /** * Add project to project hierarchy @@ -146,14 +152,27 @@ function project_hierarchy_get_parent( $p_project_id, $p_show_disabled = false ) * @return void */ function project_hierarchy_cache( $p_show_disabled = false ) { + global $g_cache_project_hierarchy_enabled, $g_cache_project_inheritance_enabled; + global $g_cache_project_hierarchy_all, $g_cache_project_inheritance_all; global $g_cache_project_hierarchy, $g_cache_project_inheritance; - global $g_cache_show_disabled; - if( !is_null( $g_cache_project_hierarchy ) && ( $g_cache_show_disabled == $p_show_disabled ) ) { + /* Should disabled projects be shown and do we already cache the whole hierarchy? */ + if( ( !is_null( $p_show_disabled ) ) && !is_null( $g_cache_project_hierarchy_all ) ) { + /* Set aliases and return */ + $g_cache_project_hierarchy = $g_cache_project_hierarchy_all; + $g_cache_project_inheritance = $g_cache_project_inheritance_all; return; } - $g_cache_show_disabled = $p_show_disabled; + /* Should disabled projects be hidden and do we already cache the enabled hierarchy? */ + if( ( is_null( $p_show_disabled ) ) && !is_null( $g_cache_project_hierarchy_enabled ) ) { + /* Set aliases and return */ + $g_cache_project_hierarchy = $g_cache_project_hierarchy_enabled; + $g_cache_project_inheritance = $g_cache_project_inheritance_enabled; + return; + } + + /* Build the missing cache */ db_param_push(); $t_enabled_clause = $p_show_disabled ? '1=1' : 'p.enabled = ' . db_param(); @@ -187,6 +206,15 @@ function project_hierarchy_cache( $p_show_disabled = false ) { $g_cache_project_inheritance[$t_project_id][$t_parent_id] = $t_parent_id; } } + + /* Copy aliases into the right pair of caches */ + if( is_null( $p_show_disabled ) ) { + $g_cache_project_hierarchy_enabled = $g_cache_project_hierarchy; + $g_cache_project_inheritance_enabled = $g_cache_project_inheritance; + } else { + $g_cache_project_hierarchy_all = $g_cache_project_hierarchy; + $g_cache_project_inheritance_all = $g_cache_project_inheritance; + } } /** -- 2.39.2 |
|
Thanks for the bug report and the patch. This needs some testing to confirm, but I already had a quick look and the proposed fix seems OK except for inline comments which should use Would you be able to submit a pull request on GitHub (https://github.com/mantisbt/mantisbt) ? This will make code review and test easier for us. |
|
I will prepare a pull request, with the requested changes. |
|
Thanks ! |
|
I tried to reproduce the performance issue on my dev box, created a parent project with 2000 linked subproject, and while the page loading is slightly slower, the degradation is nowhere as dramatic as what you describe. The page loads in 1.5 seconds, with query execution time of 0.2 seconds on average (master branch) So there is some improvement, but I'm wondering why it's taking 20 seconds for you. |
|
After more tests, the 20-odd seconds performance hit when the patch is not applied occurs consistently on a production VM, and consistently not on a development VM with identical CPU 2) and RAM (4GB) limits (but with a different host and hypervisor). When (if) I find the root cause for the performance hit I'll complement this note for the sake of completeness. EDIT: the production VM has been found to have general performance issues, which explain why it was hit much harder by the cache rebuild issue. |
|
Thanks for the feedback. Is it the same dataset (in particular the project hierarchy) between your DEV and PROD systems ? |
|
I used the production dataset during my dev. BTW: I have updated my comment about the 20-second hit. It seems like the VM has general performance issues which explain why it is hit harder by the bug. |
|
Thanks for your contribution ! |
|
MantisBT: master-2.26 5ff35d16 2024-08-02 07:40 Committer: dregad Details Diff |
Fix project_hierarchy_cache() Currently, project_hierarchy_cache() caches either all projects or enabled project only, whichever was requested last. Project edit page calls project_hierarchy_cache() for each subproject in turn, once for the whole hierarchy, and once for the enabled projects hierarchy. This causes a cache reload on each call, defeating its purpose. The code now caches both hierarchies independently, so that alternating calls will not cause cache reloads anymore. Fixes 0034526, PR https://github.com/mantisbt/mantisbt/pull/2020 Signed-off-by: Damien Regad <dregad@mantisbt.org> Original commit message edited, fixed incorrect Issue reference. |
Affected Issues 0034526 |
|
mod - core/project_hierarchy_api.php | Diff File | ||
MantisBT: master-2.26 0c73e36e 2024-09-06 18:00 Details Diff |
Fix disabled projets' inclusion in hierarchy cache The logic to check $p_show_disabled using is_null() was incorrect, causing disabled projects to be excluded from the cache when the param's value true. This resulted in disabled project not being listed on Manage Projects Page. Changing code to perform a boolean test fixes the problem. Regression introduced by 5ff35d16e85c1aa58cc80fea260cd0f3cfc39910 (issue 0034526). Fixes 0034618 |
Affected Issues 0034526, 0034618, 0034768 |
|
mod - core/project_hierarchy_api.php | Diff File |