Skip to content

Conversation

@kesselb
Copy link
Contributor

@kesselb kesselb commented Jan 30, 2019

There is no apc for PHP7+ so there is no need to check if exist.
accelerator_reset looks even more ancient.

https://en.wikipedia.org/wiki/List_of_PHP_accelerators#Compatibility_chart

Only used by writeData (e.g. some config value changed). There is no need to backport it.

There is no apc for PHP7+ so there is no need to check if exist.
accelerator_reset looks even more ancient.

Signed-off-by: Daniel Kesselberg <[email protected]>
@kesselb kesselb added 3. to review Waiting for reviews technical debt labels Jan 30, 2019
@kesselb kesselb added this to the Nextcloud 16 milestone Jan 30, 2019
@MorrisJobke
Copy link
Member

I would say: remove both methods and move them to OC\Config::writeData.

@kesselb
Copy link
Contributor Author

kesselb commented Jan 30, 2019

I would say: remove both methods and move them to OC\Config::writeData.

Sounds good.

Returns TRUE if the opcode cache for script was invalidated or if there was nothing to invalidate, or FALSE if the opcode cache is disabled.

https://secure.php.net/manual/en/function.opcache-invalidate.php

		if (function_exists('opcache_invalidate')) {
			@opcache_invalidate($this->configFilePath, true);
		}

opcache_invalidate returns false if opcache is disabled and there is no need to call opcache_reset right? So above should do the trick. Not sure if we need to call opcache_reset somewhere.

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and work

@MorrisJobke
Copy link
Member

PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 20480 bytes) in phar:///usr/local/bin/phpunit/php-code-coverage/Driver/Xdebug.php on line 86

but only in one test -> let's merge it and see if this is happening again.

@MorrisJobke MorrisJobke merged commit 5fe151f into master Feb 1, 2019
@MorrisJobke MorrisJobke deleted the remove-dead-code branch February 1, 2019 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants