Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Discussion: APC vs APCU #47

Closed
remicollet opened this issue Nov 12, 2015 · 8 comments
Closed

Discussion: APC vs APCU #47

remicollet opened this issue Nov 12, 2015 · 8 comments
Milestone

Comments

@remicollet
Copy link
Contributor

About Zend\Cache\Storage\Adapter\Apc

Starting with PHP 7, APCU won't provide the apc_* fonction by default. So the test in __construct will pass, and later other methods will fail with undefined function.

For "legacy" branch

 $enabled = ini_get('apc.enabled') && function_exists('apc_store');

For current branch which requires PHP >= 5.5, as no APC version exists I think It could make sense to use APCU by default, and switch all call to apcu_* functions.

Another way could be to create an Zend\Cache\Storage\Adapter\Apcu adapter, and mark the previous as deprecated. But it will require code changes in all consumer of this library.

Comments ?

P.S. notice, for now, apcu 5.0.0-dev segfaults during the test suite.... under investigation.

@Maks3w
Copy link
Member

Maks3w commented Nov 12, 2015

If no BC break exists I think we can modify APC class for make internal calls to APCU.

At a second step we could rename the class with the correct "APCU" name and deprecate the old name. However there is no real wins of this so this could be do later.

@remicollet
Copy link
Contributor Author

If no BC break exists I think we can modify APC class for make internal calls to APCU.

I have to code ready on my computer, I'm waiting to ba able to run the test suite, before submitting a PR
https://github.com/remicollet/zend-cache/commits/issue-apcu

@marc-mabe
Copy link
Member

@remicollet @Maks3w I would prefer to add an Apcu adapter and make the Apc adapter as deprecated as I noticed that HHVM only supports the old APC methods.

I already worked on an Apcu adapter but currently I get some strange failures on travis that I don't get on my local machine.
-> https://github.com/marc-mabe/zend-cache/tree/feature/23/src/Storage/Adapter
-> https://travis-ci.org/marc-mabe/zend-cache/builds/90415153

@marc-mabe
Copy link
Member

Link to the other discussion : #23

@remicollet
Copy link
Contributor Author

I would prefer to add an Apcu adapter

I understand, even if I will prefer something transparent for users...

@marc-mabe
Copy link
Member

To summarize as I understand:

  • The Apc adapter was originally developed to be a wrapper for the APC extension
  • Later it was going to be also a wrapper for others that support the same interface as the APC extension (APCu and HHVM)
  • HHVM has (or had) a small difference in it metadata reporting - see https://github.com/zendframework/zend-cache/blob/master/src/Storage/Adapter/Apc.php#L717
  • APCu fully supports the APC interface but only if compiled with APC compatibility
  • The APCu extension has a small incompatibility in it's interface if compiled with or without APC compatibility as it's using the same class APCIterator but with different arguments

So for me it would be 100% transparent introducing a new Apcu adapter wrapping only the new APCu extension and leaving the Apc extension as is But mark it as deprecated to inform that this adapter could be dropped in the future but not before there exists an alternative for the minimum PHP version + HHVM.

So the only question I have should the new Apcu adapter support the APCu extension including the APCIterator inconsistency or not. Currently I think it needs to be supported to make the switch to APCu simpler.

@Maks3w
Copy link
Member

Maks3w commented Nov 14, 2015

I whink there are possible two choices:

  • Wait until HHVM adopt apcu_ methods.
  • Continue and to have both adapters. I think in this case APC should extend from the new APCu

remicollet added a commit to remicollet/zend-cache that referenced this issue Nov 27, 2015
remicollet added a commit to remicollet/zend-cache that referenced this issue Nov 27, 2015
@marc-mabe
Copy link
Member

closed by 2cd809d

@marc-mabe marc-mabe added this to the 2.7.0 milestone Mar 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants