Bug #39820

unnecessary moveNode in class.t3lib_tree_pagetree_commands.php createNode()

Added by Tobias Schultheiss almost 3 years ago. Updated over 2 years ago.

Status:Resolved Start date:2012-08-15
Priority:Should have Due date:
Assigned To:- % Done:

100%

Category:Pagetree Spent time: -
Target version:-
TYPO3 Version:4.6 Is Regression:
PHP Version: Sprint Focus:
Complexity:

Description

On a Typo3 4.6.10 installation i had the problem that the creation of a new page via drag&drop always timed out. A Tracking revealed that the creation of the new page took over 100 Seconds (thats pretty ugly but another issue). I finally tracked the bad Performance to the function createNode() in t3lib/tree/pagetree/class.t3lib_tree_pagetree_commands.php.
In said function is a comparison:
if ($parentNode->getWorkspaceId() !== $targetId) {
self::moveNode($node, $targetId);
}
which fails although both ids are the same. After i did a typecast to int on both of the ids, the comparison worked, the moveNode wasn't executed anymore and the duration fell to ~10 Seconds.


Related issues

related to Core - Bug #55768: Drag & drop in Workspaces moves live page Resolved 2014-02-07

Associated revisions

Revision a4b95503
Added by Tobias Schultheiss almost 3 years ago

[BUGFIX] Pagetree - Prevent a call to moveNode while creating nodes

Change-Id: I702ccb71dcf91063012eb77091f5982661ee3afd
Fixes: #39820
Releases: 6.0, 4.7, 4.6, 4.5
Reviewed-on: http://review.typo3.org/13808
Reviewed-by: Wouter Wolters
Tested-by: Wouter Wolters
Reviewed-by: Stefan Galinski
Tested-by: Stefan Galinski

Revision dc308dee
Added by Tobias Schultheiss almost 3 years ago

[BUGFIX] Pagetree - Prevent a call to moveNode while creating nodes

Change-Id: I31ccdf629592e5185f85f217f848405817a0a079
Fixes: #39820
Releases: 6.0, 4.7, 4.6, 4.5
Reviewed-on: http://review.typo3.org/15120
Reviewed-by: Wouter Wolters
Reviewed-by: Steffen Ritter
Tested-by: Steffen Ritter

Revision d4cc0ca1
Added by Tobias Schultheiss almost 3 years ago

[BUGFIX] Pagetree - Prevent a call to moveNode while creating nodes

Change-Id: I355cfb5ed4d2ebfac26376f5f774dca4d89a0c6c
Fixes: #39820
Releases: 6.0, 4.7, 4.6, 4.5
Reviewed-on: http://review.typo3.org/15555
Reviewed-by: Wouter Wolters
Tested-by: Wouter Wolters
Reviewed-by: Stefan Neufeind
Reviewed-by: Xavier Perseguers
Tested-by: Xavier Perseguers

Revision eb24931f
Added by Tobias Schultheiss over 2 years ago

[BUGFIX] Pagetree - Prevent a call to moveNode while creating nodes

Change-Id: I355cfb5ed4d2ebfac26376f5f774dca4d89a0c6c
Fixes: #39820
Releases: 6.0, 4.7, 4.6, 4.5
Reviewed-on: https://review.typo3.org/17642
Reviewed-by: Mario Rimann
Reviewed-by: Benjamin Mack
Tested-by: Benjamin Mack

History

#1 Updated by Steffen Gebert almost 3 years ago

Oh, great improvement, Tobias! Thanks a lot.

Would you like to create a patch and send this into the review workflow? see http://wiki.typo3.org/Contribution_Walkthrough_Tutorials.

If you have any trouble, just ask!

#2 Updated by Gerrit Code Review almost 3 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13808

#3 Updated by Tobias Schultheiss almost 3 years ago

Gerrit Code Review wrote:

Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13808

Ok, first time i used git and i got mail from mr. jenkins.
what have i done wrong?

#4 Updated by Tobias Schultheiss almost 3 years ago

Tobias Schultheiss wrote:

Gerrit Code Review wrote:

Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13808

Ok, first time i used git and i got mail from mr. jenkins.
what have i done wrong?

seems i had mixed up some submodule pointers.
i made a new clone and changed the code again, this time it seems the pointers are all right.
do i just have to push the patch again?

#5 Updated by Steffen Gebert almost 3 years ago

Hi Tobias,

sorry, I was on holiday, but Michi Stucki should have taken over (he promised to substitute myself).
But thanks a lot for digging into Git, I appreciate it!

To upload an updated version, please include this line in the commit message:

Change-Id: I702ccb71dcf91063012eb77091f5982661ee3afd

Otherwise a new change request will be created (which wouldn't be too dramatic).

We have a Troubleshooting page in the wiki, which can help you with the submodule problems: http://wiki.typo3.org/TroubleShooting_(Git)

And here's a tutorial, how you can upload an updated patch set: http://wiki.typo3.org/Contribution_Walkthrough_with_CommandLine#Improving_the_patch_and_uploading_a_new_Patch_Set

If I can help you, please post again. Will be available the next days!

Steffen

#6 Updated by Michael Stucki almost 3 years ago

@Steffen: I'm looking at those issues right now, sorry that I couldn't do it before.

#7 Updated by Gerrit Code Review almost 3 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13808

#8 Updated by Gerrit Code Review almost 3 years ago

Patch set 3 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13808

#9 Updated by Tobias Schultheiss almost 3 years ago

Steffen Gebert wrote:

Hi Tobias,

sorry, I was on holiday, but Michi Stucki should have taken over (he promised to substitute myself).
But thanks a lot for digging into Git, I appreciate it!

To upload an updated version, please include this line in the commit message:
[...]

Otherwise a new change request will be created (which wouldn't be too dramatic).

We have a Troubleshooting page in the wiki, which can help you with the submodule problems: http://wiki.typo3.org/TroubleShooting_(Git)

And here's a tutorial, how you can upload an updated patch set: http://wiki.typo3.org/Contribution_Walkthrough_with_CommandLine#Improving_the_patch_and_uploading_a_new_Patch_Set

If I can help you, please post again. Will be available the next days!

Steffen

Hi Steffen,

thanks for the resources and your assistance/comments (inspired me to share ... although i had to wait for the answer; I begrudge anyone his vacation), I think I managed to push it now correctly.

#10 Updated by Gerrit Code Review almost 3 years ago

Patch set 4 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13808

#11 Updated by Gerrit Code Review almost 3 years ago

Patch set 5 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13808

#12 Updated by Gerrit Code Review almost 3 years ago

Patch set 6 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13808

#13 Updated by Gerrit Code Review almost 3 years ago

Patch set 7 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13808

#14 Updated by Gerrit Code Review almost 3 years ago

Patch set 1 for branch TYPO3_4-7 has been pushed to the review server.
It is available at http://review.typo3.org/15120

#15 Updated by Tobias Schultheiss almost 3 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#16 Updated by Stefan Galinski almost 3 years ago

  • Status changed from Resolved to Under Review

#17 Updated by Tobias Schultheiss almost 3 years ago

  • Status changed from Under Review to Resolved

#18 Updated by Gerrit Code Review almost 3 years ago

  • Status changed from Resolved to Under Review

Patch set 1 for branch TYPO3_4-6 has been pushed to the review server.
It is available at http://review.typo3.org/15555

#19 Updated by Gerrit Code Review almost 3 years ago

Patch set 2 for branch TYPO3_4-6 has been pushed to the review server.
It is available at http://review.typo3.org/15555

#20 Updated by Tobias Schultheiss almost 3 years ago

  • Status changed from Under Review to Resolved

#21 Updated by Gerrit Code Review over 2 years ago

  • Status changed from Resolved to Under Review

Patch set 1 for branch TYPO3_4-5 has been pushed to the review server.
It is available at https://review.typo3.org/17642

#22 Updated by Tobias Schultheiss over 2 years ago

  • Status changed from Under Review to Resolved

Also available in: Atom PDF