Skip to content

Conversation

@individual-it
Copy link
Member

instead of calling \OCP\Util::needUpgrade() again and again, save the result in self::$needUpgrade and return that value when asked

I have done some performance tests. Running 4 user sessions, every uploading 10 folders with 10 small files (4byte) in it. After the upload the test runs PROFIND and downloads all the files again.
Here is the code of the test: https://github.com/individual-it/owncloud-performance-test
It depends on https://github.com/owncloud/pyocclient/

I have compared (means of 4 runs):

owncloud_perf

where \OCP\Util::needUpgrade() is called we can call as well
self::checkUpgrade and use the cached result
In line 877 the call way unnecessary anyway because of the first part of
the if statement
@karlitschek
Copy link
Contributor

interesting idea. @VicDeo @icewind1991 opinions?

@individual-it
Copy link
Member Author

It is pretty much the same what @LukasReschke has done in #18914

@LukasReschke
Copy link
Member

Wouldn't it be easier to cache at \OCP\Util::needUpgrade in util.php instead?

@scrutinizer-notifier
Copy link

A new inspection was created.

@individual-it
Copy link
Member Author

Yes, you are right, thats even better. I've changed it.
Still in base.php I think the calls should be consequent all self::checkUpgrade and not mixed with \OCP\Util::needUpgrade()

@LukasReschke
Copy link
Member

👍

@VicDeo
Copy link
Member

VicDeo commented Sep 17, 2015

@karlitschek there is no milestone. 8.2 or 9?

@individual-it
Copy link
Member Author

Who is allowed to set the milestone?

@karlitschek
Copy link
Contributor

@DeepDiver1975 @PVince81 what do you guys think?

Copy link
Contributor

Choose a reason for hiding this comment

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

checkUpgrade would display an upgrade page, might be better to call needUpgrade() instead in most cases instead of the other way around ?

Actually checkUpgrade should be called only once to display the page if needed

Copy link
Member Author

Choose a reason for hiding this comment

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

checkUpgrade has the $showTemplate switch
Just tried to be consequent in the file and not mix needUpgrade() and checkUpgrade()

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I misread the code in checkUpgrade, should be safe.

@PVince81
Copy link
Contributor

👍

@DeepDiver1975 DeepDiver1975 added this to the 8.2-current milestone Sep 24, 2015
@DeepDiver1975
Copy link
Member

@individual-it You have push access to this repo - right? Please push the branch with all your changes in here - then the CI system can catch up. THX

@individual-it
Copy link
Member Author

created a new PR #19355

@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants