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 Node::getParent and usage in previews #33602

Merged
merged 3 commits into from
Aug 25, 2022
Merged

Conversation

juliushaertl
Copy link
Member

@juliushaertl juliushaertl commented Aug 18, 2022

Node::getParent does not need to query the database again if the parent node was already used before for constructing the child node. In those cases it is now passed to the child node and can be reused.

The second commit avoids using the getParent call on preview generation which saves one query.

Found in Collectives where there are similar optimisations, but this would be a generic use to help prevent this for users of getParent()

Copy link
Member

@CarlSchwan CarlSchwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clever :)

lib/private/Files/Node/Node.php Fixed Show fixed Hide fixed
lib/private/Files/Node/Node.php Fixed Show fixed Hide fixed
lib/private/Files/Node/Node.php Fixed Show fixed Hide fixed
@CarlSchwan
Copy link
Member

One failure left

  1. TrashbinTest::testExpireOldFilesUtilLimitsAreMet
    Failed asserting that 0 is identical to 10.

/home/runner/work/server/server/apps/files_trashbin/tests/TrashbinTest.php:366

@blizzz blizzz mentioned this pull request Aug 24, 2022
@juliushaertl
Copy link
Member Author

Tests revealed an additional issue for cases where we cannot just pass the current node as parent, e.g. if get('user_id/files') is called on the root folder, as then the root would be considered the parent (while /user_id would be the actual one). Pushed a possible fix, so let's see what tests tell now.

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
As the user folder might be initialized by the root from two levels
down the hierarchy, passing this as a parent only works if the path matches

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@CarlSchwan
Copy link
Member

The only faikure now is the one about session that is unrelated

@juliushaertl
Copy link
Member Author

Fix for the failing session test is in #33668

@juliushaertl juliushaertl added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 24, 2022
@juliushaertl juliushaertl merged commit 06340b6 into master Aug 25, 2022
@juliushaertl juliushaertl deleted the perf/parent-node branch August 25, 2022 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish performance 🚀
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants