View Issue Details

IDProjectCategoryView StatusLast Update
0034526mantisbtperformancepublic2024-09-27 01:21
Reporteraaribaud-fgs Assigned Tocommunity  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Target Version2.26.3Fixed in Version2.26.3 
Summary0034526: 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.
Going to "Manage", "Projects", and clicking on the parent project (for instance, to add a new sub-project to it), the manage_proj_edit_page.php- page takes about 20 seconds to show.

Steps To Reproduce

Create a (parent) project.
Create 1700 sub-projects under the parent project.
Open the parent project edition page.
See that loading this page is much slower than with only a few projects.

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:

  • 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.

TagsNo tags attached.

Relationships

related to 0034618 closeddregad Disabled projects are not listed on page manage_proj_page.php 
related to 0034768 closedatrol 'INTERNAL APPLICATION ERROR' editing some projects from manage_proj_page.php 

Activities

aaribaud-fgs

aaribaud-fgs

2024-08-02 07:43

reporter   ~0069061

Last edited: 2024-08-02 07:43

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

dregad

dregad

2024-08-06 05:31

developer   ~0069075

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 # xxx syntax instead of /* xxx */. Also the commit message has long lines which should be wrapped [1], and it misses the Issue reference (i.e. Fixes #345226)

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.

aaribaud-fgs

aaribaud-fgs

2024-08-06 11:14

reporter   ~0069081

I will prepare a pull request, with the requested changes.

dregad

dregad

2024-08-06 11:54

developer   ~0069083

Thanks !

aaribaud-fgs

aaribaud-fgs

2024-08-06 15:27

reporter   ~0069084

PR done at https://github.com/mantisbt/mantisbt/pull/2020

dregad

dregad

2024-08-09 13:58

developer   ~0069087

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)
With your patch, page load is 1.3 seconds, query execution 0.1 seconds

So there is some improvement, but I'm wondering why it's taking 20 seconds for you.

aaribaud-fgs

aaribaud-fgs

2024-08-13 03:35

reporter   ~0069089

Last edited: 2024-08-19 02:54

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.

dregad

dregad

2024-08-13 05:29

developer   ~0069090

Thanks for the feedback. Is it the same dataset (in particular the project hierarchy) between your DEV and PROD systems ?

aaribaud-fgs

aaribaud-fgs

2024-08-19 02:55

reporter   ~0069114

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.

dregad

dregad

2024-08-19 12:43

developer   ~0069115

Thanks for your contribution !

Related Changesets

MantisBT: master-2.26 5ff35d16

2024-08-02 07:40

aaribaud-fgs

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

dregad


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