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

Remove children from the cache in one query #13394

Merged
merged 2 commits into from Jan 15, 2015

Conversation

Projects
None yet
7 participants
@icewind1991
Copy link
Member

icewind1991 commented Jan 15, 2015

Go from 1 query per folder + 1 query per file to 2 queries per folder

cc @PVince81 @DeepDiver1975

@owncloud-bot

This comment has been minimized.

Copy link
Contributor

owncloud-bot commented Jan 15, 2015

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/6985/
👍 Test PASSed. 👍

@PVince81

This comment has been minimized.

Copy link
Member

PVince81 commented Jan 15, 2015

Can you extend "testFolder" to have a second level of folders ?
Currently the test folder only contains two files "foo" and "bar" and doesn't cover the case where we recurse into subdirs.

@icewind1991

This comment has been minimized.

Copy link
Member Author

icewind1991 commented Jan 15, 2015

Added extra unit test

@MorrisJobke

This comment has been minimized.

Copy link
Member

MorrisJobke commented Jan 15, 2015

Awesome. It's much faster 👍

1GB in 136 files

11.1s -> 809 ms (with disabled trashbin and encryption)

Before https://blackfire.io/profiles/db8a168e-41b3-4de7-8f02-63023b4a434b/graph
After https://blackfire.io/profiles/885db930-dea5-40b9-9366-d1fe144f72de/graph

@MorrisJobke

This comment has been minimized.

Copy link
Member

MorrisJobke commented Jan 15, 2015

before 136 * OC_DB::executeAudited
after 7 * OC_DB::executeAudited

@MorrisJobke

This comment has been minimized.

@owncloud-bot

This comment has been minimized.

Copy link
Contributor

owncloud-bot commented Jan 15, 2015

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/6999/
👍 Test PASSed. 👍

@PVince81

This comment has been minimized.

Copy link
Member

PVince81 commented Jan 15, 2015

Code looks good, makes sense 👍

@PVince81

This comment has been minimized.

Copy link
Member

PVince81 commented Jan 15, 2015

@DeepDiver1975 if this is ok for 8.0 then we can merge this ?

@MorrisJobke

This comment has been minimized.

Copy link
Member

MorrisJobke commented Jan 15, 2015

Even more impressive is the deletion of 2.2 GB with nearly 700 files and process time drops from 52.9s to 1.63s \o/ I like that profiling :)

@DeepDiver1975

This comment has been minimized.

Copy link
Member

DeepDiver1975 commented Jan 15, 2015

@DeepDiver1975 if this is ok for 8.0 then we can merge this ?

let me have a look ... had some reallife the past few houes 😉

@DeepDiver1975

This comment has been minimized.

Copy link
Member

DeepDiver1975 commented Jan 15, 2015

@MorrisJobke can you please share the profiling graph with me? THX

@LukasReschke

This comment has been minimized.

Copy link
Member

LukasReschke commented Jan 15, 2015

@MorrisJobke can you please share the profiling graph with me? THX

They are posted above? - The links in #13394 (comment) work fine for me here

@DeepDiver1975

This comment has been minimized.

Copy link
Member

DeepDiver1975 commented Jan 15, 2015

They are posted above? - The links in #13394 (comment) work fine for me here

thx - the compare did not work .... anyway

@DeepDiver1975 DeepDiver1975 added this to the 8.0-current milestone Jan 15, 2015

@DeepDiver1975

This comment has been minimized.

Copy link
Member

DeepDiver1975 commented Jan 15, 2015

@DeepDiver1975 if this is ok for 8.0 then we can merge this ?

🚀 THX guys! - Now that you know how it works: GO GO GO 😉

DeepDiver1975 added a commit that referenced this pull request Jan 15, 2015

Merge pull request #13394 from owncloud/cache-remove-folder
Remove children from the cache in one query

@DeepDiver1975 DeepDiver1975 merged commit 0ebcc2d into master Jan 15, 2015

1 check passed

default Merged build finished.
Details

@DeepDiver1975 DeepDiver1975 deleted the cache-remove-folder branch Jan 15, 2015

@MorrisJobke

This comment has been minimized.

@MorrisJobke

This comment has been minimized.

Copy link
Member

MorrisJobke commented Jan 15, 2015

thx - the compare did not work .... anyway

Upps ... wrong link. I updated it above.

https://blackfire.io/profiles/compare/97434597-dc19-4756-a76f-bdde8647a36e/graph

@DeepDiver1975

This comment has been minimized.

Copy link
Member

DeepDiver1975 commented Jan 15, 2015

@icewind1991 nice job - THX

@scrutinizer-notifier

This comment has been minimized.

Copy link

scrutinizer-notifier commented Jan 15, 2015

The inspection completed: 7 new issues, 4 updated code elements

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment