increase default opcache memory buffers/limits #2291
Conversation
|
failing travis build looks unrelated: |
|
Merged b655f2d Thanks. |
|
Looks to me like Travis is running against the .travis.yml of master rather than PHP-7.0 |
|
I don't think this should have been merged into PHP-7.0. We shouldn't change ini settings in the middle of a release. If people left some opcache setting on the default value doing a patch version upgrade should not suddenly double their memory usage or similar. |
|
@krakjoe Realpath cache is an upper limit, while this involves preallocation, i.e. it will definitely increase memory usage if defaults are used. I don't see a good reason why these changes need to be in 7.0 (this is not a bug fix). |
|
@nikic waiting for 7.2 doesn't seem ideal either, 7.1 is also mid release ... |
|
@nikic, @krakjoe thanks for the ping. I'd be for not merging into 7.0 as well. Till now the issue handling in 7.0 has been quite permissive. This was on purpose and justified by the facts, that 7.0 was a new major version, but also with an in many ways divergent code base, so still had/has some porting debt and revealed many unmaintained extensions that needed to be synched. For this reason, many patches was accepted, that wouldn't have been otherwise. Just to mention - changes to pdo_dblib, pdo_firebird, several new ini options and even some unavoidable fixes possibly breaching things. I think, it is now a good time to get more conservative about the 7.0 branch. Given 7.0 is now one year old, is contained in some distros by default, and its usage increases as shown by some stats. OFC we're still doing active bug fixing and even can add some tiny new feature, but in general it's now really more about having things reliable and more stable. There was some cases really at the border of the release process rules, we should now only accept cases clearly under that line. Maybe I should send a notification to internals, suggesting this. With changing the default settings in the core, not in INI - it sounds indeed unsafe. There's no guarantee every INI setting is set explicitly by admins, so changing the default in the core can cause any possible issue, or at least false positives about increased memory, etc. Those who care about the particular INI settings, most likely already have them set explicitly, those who don't should not experience a sudden change. With the realpath cache, FYI I was also doing some rework in master to reduce the size of the structs, to probabl an increase would be more suitable only there, but 7.1 is still very new so would fit there somehow as well. But in any case, I think the changes to the default core INI items should be reverted from 7.0, while changes to the ini files were ok. Going by the middle ground of 7.1 sounds plausible, as for me. Thanks. |
|
Fair enough @weltling, reverted from 7.0 I think it's okay to leave in 7.1, since we are so early in the release cycle. |
|
I am fine with whatever you guys think fits best |
as discussed in #2238 (comment)