Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [RFC] 64 bit platform improvements for string length and integer »

[RFC] 64 bit platform improvements for string length and integer

php.internals

Anatoliy Belsky

12 years ago
Hi, https://wiki.php.net/rfc/size_t_and_int64 The discussion time has come. The work on the feature branch continues. The current patch is stable enough to be discussed. Regards Anatol

Kalle Sommer Nielsen

12 years ago
Hi Anatol 2014/1/10 Anatol Belski <ab@php.net>:
> Hi, > > https://wiki.php.net/rfc/size_t_and_int64
I absolutely love the work, time and effort you have put in to this branch, I've been following it closely on the sideline. There is one thing I'm wondering about, maybe I skipped through some of the sections too fast in the RFC, but what about API BC? I know you propose it for PHP6, but are there gonna be any macros or other helpers for extension developers to ease the use of #ifdefs, I realize things like the parameter parsing one is gonna be tough, but just as a general thought, I think Derick was asking something like this not too long ago?
-- regards, Kalle Sommer Nielsen kalle@php.net

Anatoliy Belsky

12 years ago
Hi Kalle, On Fri, January 10, 2014 19:29, Kalle Sommer Nielsen wrote:
> Hi Anatol > > > 2014/1/10 Anatol Belski <ab@php.net>: > >> Hi, >> >> >> https://wiki.php.net/rfc/size_t_and_int64 >> > > I absolutely love the work, time and effort you have put in to this > branch, I've been following it closely on the sideline. There is one thing > I'm wondering about, maybe I skipped through some of the > sections too fast in the RFC, but what about API BC? I know you propose it > for PHP6, but are there gonna be any macros or other helpers for extension > developers to ease the use of #ifdefs, I realize things like the parameter > parsing one is gonna be tough, but just as a general thought, I think > Derick was asking something like this not too > long ago? >
thanks for the good words :) The short answer is: yes. I gonna start with that probably right as next. In plan is the tool for any possible automatic replacements and the header(s) for backward compatibility. That will be a big ease for the first step, though it can of course not replace the manual porting. Parameter parsing is one of the cracky points, but more it will be about the extension code adoption. Especially size_t usage, while being trivial, might get one in a muddle. I personally needed 1-2 hours to habituate size_t, so it works. Cheers anatol

Anatoliy Belsky

12 years ago
On Fri, January 10, 2014 20:32, Anatol Belski wrote:
> Hi Kalle, > > > On Fri, January 10, 2014 19:29, Kalle Sommer Nielsen wrote: > >> Hi Anatol >> >> >> >> 2014/1/10 Anatol Belski <ab@php.net>: >> >> >>> Hi, >>> >>> >>> >>> https://wiki.php.net/rfc/size_t_and_int64 >>> >>> >> >> I absolutely love the work, time and effort you have put in to this >> branch, I've been following it closely on the sideline. There is one >> thing I'm wondering about, maybe I skipped through some of the >> sections too fast in the RFC, but what about API BC? I know you propose >> it for PHP6, but are there gonna be any macros or other helpers for >> extension developers to ease the use of #ifdefs, I realize things like >> the parameter parsing one is gonna be tough, but just as a general >> thought, I think Derick was asking something like this not too >> long ago? >> > thanks for the good words :) > > The short answer is: yes. I gonna start with that probably right as next. > In plan is the tool for any possible automatic replacements and the > header(s) for backward compatibility. That will be a big ease for the first > step, though it can of course not replace the manual porting. Parameter > parsing is one of the cracky points, but more it will be about the > extension code adoption. Especially size_t usage, while being trivial, > might get one in a muddle. I personally needed 1-2 hours to habituate > size_t, so it works. >
Ah, btw it is on the RFC, a couple of sentences under "Migration path for PECL extensions". Anatol

Jan Ehrhardt

12 years ago
"Anatol Belski" in php.internals (Fri, 10 Jan 2014 15:58:23 +0100):
>https://wiki.php.net/rfc/size_t_and_int64 > >The discussion time has come. The work on the feature branch continues. >The current patch is stable enough to be discussed.
I do not have voting karma, but I can confirm that the current experimental branch is quite stable. I have been testing the experimental builds for Windows (and compiling them myself). In Drupal6, Drupal7, Wordpress and own own code I am getting very few different results compared to the 5.5 releases. I ran into a segfault with one Drupal7 site (but not with other Drupal7 sites) with opcache enabled. But upon investigating that glitch, I discovered that PHP 5.5 NTS stumbled over the same Drupal7 site with opcache enabled. See the php-windows list for details. Keep on the good work, Anatol! Jan

Nikita Popov

12 years ago
On Fri, Jan 10, 2014 at 3:58 PM, Anatol Belski <ab@php.net> wrote:
> Hi, > > https://wiki.php.net/rfc/size_t_and_int64 > > The discussion time has come. The work on the feature branch continues. > The current patch is stable enough to be discussed. >
What is the reason behind the renames of IS_LONG to IS_INT (and Z_LVAL to Z_IVAL etc), as well as the renames in zpp (s -> S, etc)? Why can't we keep the old names here? That should reduce the amount of ifndefs involved a lot, as you'd only have to do it for the type declarations themselves, not for every single usage. Or is the point here to intentionally provide a maximum amount of BC breakage, so code doesn't "accidentally" continue to run (e.g. I think that without the renames extensions could continue to run mostly without issue on 32bit.) Nikita

Anatoliy Belsky

12 years ago
Hi Nikita, On Fri, January 10, 2014 22:42, Nikita Popov wrote:
> On Fri, Jan 10, 2014 at 3:58 PM, Anatol Belski <ab@php.net> wrote: > > >> Hi, >> >> >> https://wiki.php.net/rfc/size_t_and_int64 >> >> >> The discussion time has come. The work on the feature branch continues. >> The current patch is stable enough to be discussed. >> >> > > What is the reason behind the renames of IS_LONG to IS_INT (and Z_LVAL to > Z_IVAL etc), as well as the renames in zpp (s -> S, etc)? Why can't we > keep the old names here? That should reduce the amount of ifndefs involved > a lot, as you'd only have to do it for the type declarations themselves, > not for every single usage. Or is the point here to intentionally provide > a maximum amount of BC breakage, so code doesn't "accidentally" continue > to run (e.g. I think that without the renames extensions could continue to > run mostly without issue on 32bit.) > > Nikita >
the renames you mention like IS_LONG -> IS_INT are thought more for correct semantic, as there is no firm 'long' anymore. The same for Z_STRLEN -> Z_STRSIZE and others. That kind of thing should be done the most obvious way. Well, the max BC reason you gave I like too :) For the same reason zend_parse_parameters() formats was changed, as it'll issue an error on runtime. However here I still scratch my head as that's a runtime issue, but it should break the compilation as well. Without semantic replacements many extensions would just continue to compile/run on 32 bit, indeed. Though because of the size_t one could still have some unpleasant surprise in some situation. Besides that, I'd really see that more like a side effect with not very clear use. With the #ifdef's - there shouldn't be any or should be very few. The compatibility header I've mentioned in one of the previous responses should do it one for all. It can look like #if PHP_MAJOR_VERSION < 6 # define IS_INT IS_LONG # define php_size_t int ....... #endif Once included and given an extension is compiled with an older PHP version, that defines should cover the compatibility with older semantics. Some more complicated solution will probably be needed for zpp to replace "i" with "l" and co. for the older PHP. But generally, such a header should make the same ext source in 6.x style compatible with 5.x branch. Of course some exceptions will have to take place, but i think those will be not more than the current mainstream already contains to separate the code for minor versions in 5.x branch. The migration path is the very next thing I have to do. Regards Anatol

Hannes Magnusson

12 years ago
On Fri, Jan 10, 2014 at 3:11 PM, Anatol Belski <ab@php.net> wrote:
> Hi Nikita, > > On Fri, January 10, 2014 22:42, Nikita Popov wrote: >> On Fri, Jan 10, 2014 at 3:58 PM, Anatol Belski <ab@php.net> wrote: >> >> >>> Hi, >>> >>> >>> https://wiki.php.net/rfc/size_t_and_int64 >>> >>> >>> The discussion time has come. The work on the feature branch continues. >>> The current patch is stable enough to be discussed. >>> >>> >> >> What is the reason behind the renames of IS_LONG to IS_INT (and Z_LVAL to >> Z_IVAL etc), as well as the renames in zpp (s -> S, etc)? Why can't we >> keep the old names here? That should reduce the amount of ifndefs involved >> a lot, as you'd only have to do it for the type declarations themselves, >> not for every single usage. Or is the point here to intentionally provide >> a maximum amount of BC breakage, so code doesn't "accidentally" continue >> to run (e.g. I think that without the renames extensions could continue to >> run mostly without issue on 32bit.) >> >> Nikita >> > the renames you mention like IS_LONG -> IS_INT are thought more for > correct semantic, as there is no firm 'long' anymore. The same for > Z_STRLEN -> Z_STRSIZE and others. That kind of thing should be done the > most obvious way. Well, the max BC reason you gave I like too :) > > For the same reason zend_parse_parameters() formats was changed, as it'll > issue an error on runtime. However here I still scratch my head as that's > a runtime issue, but it should break the compilation as well. > > Without semantic replacements many extensions would just continue to > compile/run on 32 bit, indeed. Though because of the size_t one could > still have some unpleasant surprise in some situation. Besides that, I'd > really see that more like a side effect with not very clear use. > > With the #ifdef's - there shouldn't be any or should be very few. The > compatibility header I've mentioned in one of the previous responses > should do it one for all. It can look like > > #if PHP_MAJOR_VERSION < 6 > # define IS_INT IS_LONG > # define php_size_t int > ....... > #endif > > Once included and given an extension is compiled with an older PHP > version, that defines should cover the compatibility with older semantics. > Some more complicated solution will probably be needed for zpp to replace > "i" with "l" and co. for the older PHP. But generally, such a header > should make the same ext source in 6.x style compatible with 5.x branch. > Of course some exceptions will have to take place, but i think those will > be not more than the current mainstream already contains to separate the > code for minor versions in 5.x branch. > > The migration path is the very next thing I have to do.
I am really confused on that. How is the compatibility header useful if I still need a ifdef else for zpp and things that boil down to printfs, like error reporting, with 2 different arguments? Wouldn't it be better to not change them, but maybe force extensions to define a _I_SUPPORT_PHP6, if not defined refuse to build the ext against PHP6? Then all I have to do in extension is to include the compat header, change some of my types passed to zpp/printfs and define that macro.. and I could support PHP5 and PHP6. And if I try to build against PHP6 it would result in compiler failure... We'd maybe need to tweak the build system a tiny bit, and phpize, for that to happen - but that seem to be much easier then supporting PHP5 and 6 for extensions in the same codebase with renamed zpp values? -Hannes

Anatoliy Belsky

12 years ago
Hi Hannes, On Sat, January 11, 2014 02:33, Hannes Magnusson wrote:
> On Fri, Jan 10, 2014 at 3:11 PM, Anatol Belski <ab@php.net> wrote: > >> Hi Nikita, >> >> >> On Fri, January 10, 2014 22:42, Nikita Popov wrote: >> >>> On Fri, Jan 10, 2014 at 3:58 PM, Anatol Belski <ab@php.net> wrote: >>> >>> >>> >>>> Hi, >>>> >>>> >>>> >>>> https://wiki.php.net/rfc/size_t_and_int64 >>>> >>>> >>>> >>>> The discussion time has come. The work on the feature branch >>>> continues. The current patch is stable enough to be discussed. >>>> >>>> >>>> >>> >>> What is the reason behind the renames of IS_LONG to IS_INT (and >>> Z_LVAL to >>> Z_IVAL etc), as well as the renames in zpp (s -> S, etc)? Why can't we >>> keep the old names here? That should reduce the amount of ifndefs >>> involved a lot, as you'd only have to do it for the type declarations >>> themselves, not for every single usage. Or is the point here to >>> intentionally provide a maximum amount of BC breakage, so code doesn't >>> "accidentally" continue >>> to run (e.g. I think that without the renames extensions could >>> continue to run mostly without issue on 32bit.) >>> >>> Nikita >>> >>> >> the renames you mention like IS_LONG -> IS_INT are thought more for >> correct semantic, as there is no firm 'long' anymore. The same for >> Z_STRLEN -> Z_STRSIZE and others. That kind of thing should be done the >> most obvious way. Well, the max BC reason you gave I like too :) >> >> For the same reason zend_parse_parameters() formats was changed, as >> it'll issue an error on runtime. However here I still scratch my head as >> that's a runtime issue, but it should break the compilation as well. >> >> Without semantic replacements many extensions would just continue to >> compile/run on 32 bit, indeed. Though because of the size_t one could >> still have some unpleasant surprise in some situation. Besides that, >> I'd >> really see that more like a side effect with not very clear use. >> >> With the #ifdef's - there shouldn't be any or should be very few. The >> compatibility header I've mentioned in one of the previous responses >> should do it one for all. It can look like >> >> #if PHP_MAJOR_VERSION < 6 >> # define IS_INT IS_LONG >> # define php_size_t int >> ....... >> #endif >> >> >> Once included and given an extension is compiled with an older PHP >> version, that defines should cover the compatibility with older >> semantics. Some more complicated solution will probably be needed for >> zpp to replace "i" with "l" and co. for the older PHP. But generally, >> such a header should make the same ext source in 6.x style compatible >> with 5.x branch. Of course some exceptions will have to take place, but >> i think those will be not more than the current mainstream already >> contains to separate the code for minor versions in 5.x branch. >> >> The migration path is the very next thing I have to do. >> > > > I am really confused on that. How is the compatibility header useful > if I still need a ifdef else for zpp and things that boil down to printfs, > like error reporting, with 2 different arguments?
with the php_error_docref() - very good point, the header can't help much. Except to define something like php_error_docref_comp() as a wrapper function in the compat header, so %pd and alike can be replaced on runtime. The code might miss some #ifdefs then, so be cleaner.
> Wouldn't it be better to not change them, but maybe force extensions > to define a _I_SUPPORT_PHP6, if not defined refuse to build the ext against > PHP6? >
Yeah, I'd say an explicit define to indicate PHP6 readiness is good point as well. About zpp I've got a click - the compatibility with 5.x can be integrated into zpp itself in 6.x. I mean look here http://git.php.net/?p=php-src.git;a=blob;f=Zend/zend_API.c;hb=refs/heads/str_size_and_int64#l326 - 'l' and 'L' was replaced with 'i' and 'I', but nothing prevents to turn those formats as aliases in 6.x. That way 'l' and 'i' and another pairs will do the same thing, new implementations can use clean semantics, and the old formats can be removed after 5.x EOL. This solution however won't force the new semantic. Another way i could suggest for zpp is similarly to docrefs using a wrapper like zend_parse_parameters_comp(), which would replace new for old. Still not very nice but would guarantee semantically correct formats.
> Then all I have to do in extension is to include the compat header, > change some of my types passed to zpp/printfs and define that macro.. and I > could support PHP5 and PHP6. > > And if I try to build against PHP6 it would result in compiler failure... > We'd maybe need to tweak the build system a tiny bit, and phpize, for > that to happen - but that seem to be much easier then supporting PHP5 and 6 > for extensions in the same codebase with renamed zpp values? >
Sounds plausible to me. Parameters parsing and spprintf are the only cracky places I can think of now, the other macros/function renames should be easy covered with the compat header, so can me machinable replaced with some tool, even sed script. The _I_SUPPORT_PHP6 def (should be defined before includes) can be checked directly in php.h, where an #error can be thrown. That sounds like a sane migration workflow. Like if I'm not aware of 6.x and try to compile just right on, it'll reject. If I explicitly say, that i'm ready for 6.x, means i've done the porting work before. So for the migration it'd be like - run the replacement tool on the sources for simple semantic fixes - include compat header - #define PHP6_SUPPORTED - replace appropriate things with *_compat() wrappers or do some #ifdefs - fix datatypes for function args - fix the extension code where appropriate (that might be big or not, as Jan meant some exts he's tried was easy) - check if it plays good with some lib i link against, do some range checks eventually Sounds like a plan. Thanks for the ideas! Anatol

Jakub Zelenka

12 years ago
Hi Anatol,
> > About zpp I've got a click - the compatibility with 5.x can be integrated > into zpp itself in 6.x. I mean look here > > http://git.php.net/?p=php-src.git;a=blob;f=Zend/zend_API.c;hb=refs/heads/str_size_and_int64#l326 > - 'l' and 'L' was replaced with 'i' and 'I', but nothing prevents to turn > those formats as aliases in 6.x. That way 'l' and 'i' and another pairs > will do the same thing, new implementations can use clean semantics, and > the old formats can be removed after 5.x EOL. This solution however won't > force the new semantic. >
I agree with keeping old flags ('l', 'p', 's'...). I think that would be good to add range checks to zend_parse_arg_impl for these flags. If the casted value is bigger (LONG_MAX, INT_MAX...), then warning. It would be really helpful and simplify the migration for library wrappers where these checks needs to be usually done... Thanks Jakub

Anatoliy Belsky

12 years ago
Hi Jakub, On Sat, January 11, 2014 14:18, Jakub Zelenka wrote:
> Hi Anatol, > > > > >> >> About zpp I've got a click - the compatibility with 5.x can be >> integrated into zpp itself in 6.x. I mean look here >> >> http://git.php.net/?p=php-src.git;a=blob;f=Zend/zend_API.c;hb=refs/head >> s/str_size_and_int64#l326 - 'l' and 'L' was replaced with 'i' and 'I', >> but nothing prevents to turn those formats as aliases in 6.x. That way >> 'l' and 'i' and another pairs >> will do the same thing, new implementations can use clean semantics, and >> the old formats can be removed after 5.x EOL. This solution however >> won't force the new semantic. >> > > I agree with keeping old flags ('l', 'p', 's'...). I think that would be > good to add range checks to zend_parse_arg_impl for these flags. If the > casted value is bigger (LONG_MAX, INT_MAX...), then warning. It would be > really helpful and simplify the migration for library wrappers where > these checks needs to be usually done... >
the range checks is what 'L' vs 'I' would stand for, the 'L' would be available as alias for 'I', so one don't has to change zpp. A good thing were to add type checks, with gcc http://gcc.gnu.org/onlinedocs/gcc/Typeof.html , however not sure it's available with Visual Studio. Regards Anatol

Jakub Zelenka

12 years ago
On Sat, Jan 11, 2014 at 2:48 PM, Anatol Belski <ab@php.net> wrote:
> the range checks is what 'L' vs 'I' would stand for, the 'L' would be > available as alias for 'I', so one don't has to change zpp. A good thing > were to add type checks, > >
Oh I see. The only small objection is that these lines could lead to the unexpected result from the user point of view: if (d > ZEND_INT_MAX) { *p = ZEND_INT_MAX; break; } else if (d < ZEND_INT_MIN) { *p = ZEND_INT_MIN; break; } I think that would make sense to add warning here Also similar checks for "s" and "p" length part (size_t -> int) would be great too! ;) Thanks Regards Jakub

Anatoliy Belsky

12 years ago
On Sat, January 11, 2014 16:05, Jakub Zelenka wrote:
> On Sat, Jan 11, 2014 at 2:48 PM, Anatol Belski <ab@php.net> wrote: > > >> the range checks is what 'L' vs 'I' would stand for, the 'L' would be >> available as alias for 'I', so one don't has to change zpp. A good >> thing were to add type checks, >> >> > Oh I see. The only small objection is that these lines could lead to the > unexpected result from the user point of view: > > if (d > ZEND_INT_MAX) { *p = ZEND_INT_MAX; > break; } else if (d < ZEND_INT_MIN) { > *p = ZEND_INT_MIN; > break; } > > > I think that would make sense to add warning here > > > Also similar checks for "s" and "p" length part (size_t -> int) would be > great too! ;) >
yep, that's exactly how it performs till now in the mainstream, just replace ZEND_INT_MAX with LONG_MAX. Adding a warning to every such case would cause a warning flood in many PHP apps, guaranteed :) Whereby it might be ok for debug mode maybe, I wouldn't do it as I can't remember any WTFs about the behavior. Cheers Anatol

Jakub Zelenka

12 years ago
On Sat, Jan 11, 2014 at 3:29 PM, Anatol Belski <ab@php.net> wrote:
> > yep, that's exactly how it performs till now in the mainstream, just > replace ZEND_INT_MAX with LONG_MAX. Adding a warning to every such case > would cause a warning flood in many PHP apps, guaranteed :) Whereby it > might be ok for debug mode maybe, I wouldn't do it as I can't remember any > WTFs about the behavior. > >
I meant adding warning only for overflow cases. Values in existing apps can't be bigger than LONG_MAX so I don't see how it could cause warning flood in the current PHP apps. I don't even think that users will be often using such a big values after the 64bit changes. The thing is that if I use value bigger than LONG_MAX after 64bit changes and I pass it to the function defined in extension that does not support it (use "l" in zpp), then I rather see warning than unexpected rounding to LONG_MAX... There is no way how to find out (except looking to the ext sources) that the big values are not supported. If I get warning, I can fix it in the code straight away... The another thing is that this checking needs to be done by other libraries wrappers after getting parameters because the most libraries using smaller types. That was exactly what I did when I worked on openssl ext for 64bit and I will need to do it in my fann and crypto extension. There are bunch of other extensions that will need to do it too (for example imagick). The difference with openssl changes is that the BC will need to be kept. I am sure that if warnings were already in zend_parse_arg_impl, then the work for supporting 64bit branch would much easier for me and other PECL extension maintainers... Cheers Jakub

Anatoliy Belsky

12 years ago
Hi, On Sat, January 11, 2014 04:34, Anatol Belski wrote:
> About zpp I've got a click - the compatibility with 5.x can be integrated > into zpp itself in 6.x. I mean look here > http://git.php.net/?p=php-src.git;a=blob;f=Zend/zend_API.c;hb=refs/heads/ > str_size_and_int64#l326 - 'l' and 'L' was replaced with 'i' and 'I', but > nothing prevents to turn those formats as aliases in 6.x. That way 'l' and > 'i' and another pairs > will do the same thing, new implementations can use clean semantics, and > the old formats can be removed after 5.x EOL. This solution however won't > force the new semantic. > > Another way i could suggest for zpp is similarly to docrefs using a > wrapper like zend_parse_parameters_comp(), which would replace new for old. > Still not very nice but would guarantee semantically correct formats.
Here's the first worky header and replacement tool variant http://git.php.net/?p=php-src.git;a=tree;f=compat;h=166ac434f01d38119eb12160f2513cfe99c39b42;hb=refs/heads/str_size_and_int64 . The PECL readme is in progress. After trying some extensions I come to conclusion, that both of the compatibility variants above are not good. The first one with aliasing gives no good feeling, after trying in practice - it might lead even to more confusion. The second with the *_compat() - technically it's a bit rubbish, as the specs are literals saved in the readonly TEXT segment, so replacing on the fly costs some allocations on the heap. While still doable, the overhead isn't worthy. The variant I would suggest is using ternary operator, passing both old and new style, like COMPAT ? "lLps" : "iIPS" While being simple, this makes the difference good visible, so is clear. Comparing with the breakages we had in the past like literals between 5.3 and 5.4, syntactically no #ifdef needed as it's done in place. For the printf formats I'd suggest using ZEND_INT_FMT macros concatenated with the format literal. The quirk here is the same as it's in the TEXT segment. But still, with this solution no #ifdef is needed. The latter solution suggestions are much simpler and don't need to be done much, what do you think? The readme is still in progress, however if curious, the current replacement tool and header already do the job, so one can try the porting with minimal effort. Regards Anatol

Anatoliy Belsky

12 years ago
Hi, the usable porting doc is now available http://git.php.net/?p=php-src.git;a=blob;f=compat/PECL_PORTING;hb=refs/heads/str_size_and_int64 The replacement tool and the header are in the same folder. Regards Anatol

Jan Ehrhardt

12 years ago
Nikita Popov in php.internals (Fri, 10 Jan 2014 22:42:49 +0100):
> Or is the point here to intentionally provide a >maximum amount of BC breakage, so code doesn't "accidentally" continue to >run (e.g. I think that without the renames extensions could continue to run >mostly without issue on 32bit.)
Some of the PECL extensions compiled without really complaining with the new experimental branch (on Windows). Anatol's own xmldiff, php_blenc.dll (but it did not work), php_uploadprogress.dll and php_timezonedb.dll. The BC break should probably be even greater, so that cases like the blenc extension do not occur. I have converted one of my own extensions (that is not even on PECL): php_ffmpeg.dll. I did not do that (yet) with ifndef's, but just with global searching and replacing. So I have two sources for the same extension right now. Jan

Dmitry Stogov

12 years ago
I completely agree with Nikita. Why to rename LONG->INT STRLEN->STRSIZE in thousands places? Why not just define zend_long and zend_ulong to be 64-bit on 64-bit platforms and use them instead of int, ulint, zend_int, zend_uint, long, ulong where it's necessary. Anatol, I understood your point about catching incompatibility code at compile-time, but I'm not sure if the new features cost such huge code base changes. 1) 64-bit integers on Windows (they are already 64-bit on other systems) 2) 64-bit string length. I don't think many people are interested in that. Fortunately, the patch doesn't change the zval size, so it shouldn't make a lot of harm. However, usage of "zend_size_t" instead of "int" is a bit annoying. I would change it into the same "zend_long" or "zend_ulong". Thanks. Dmitry. On Sat, Jan 11, 2014 at 1:42 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:

Pierre Joye

12 years ago
On Thu, Jan 23, 2014 at 9:42 AM, Dmitry Stogov <dmitry@zend.com> wrote:
> I completely agree with Nikita. > Why to rename LONG->INT STRLEN->STRSIZE in thousands places? > Why not just define zend_long and zend_ulong to be 64-bit on 64-bit > platforms and use them instead of int, ulint, zend_int, zend_uint, long, > ulong where it's necessary. > > Anatol, I understood your point about catching incompatibility code at > compile-time, but I'm not sure if the new features cost such huge code base > changes.
To catch 64bit issues at compile is immensely valuable. We had so many issues in the past, some of them leading to security issues. It is also a one time job, with a little extra effort for two years. Besides the portability improvements, code review and correctness is one of the goal of this RFC. I can't remember to know any widely known and sane project relying on on other type for buffer length. PHP does not have to be different :)
> 1) 64-bit integers on Windows (they are already 64-bit on other systems)
Almost all :)
> 2) 64-bit string length. I don't think many people are interested in that. > Fortunately, the patch doesn't change the zval size, so it shouldn't make a > lot of harm. However, usage of "zend_size_t" instead of "int" is a bit > annoying. I would change it into the same "zend_long" or "zend_ulong".
It is not about being interested but preventing many security issues as well, by default. The large buffer availability is a good side effect. The implementation correctness is also drastically improved in this case. Cheers,
-- Pierre @pierrejoye | http://www.libgd.org

Pierre Joye

12 years ago
re, On Thu, Jan 23, 2014 at 10:13 AM, Pierre Joye <pierre.php@gmail.com> wrote:
> On Thu, Jan 23, 2014 at 9:42 AM, Dmitry Stogov <dmitry@zend.com> wrote: >> I completely agree with Nikita. >> Why to rename LONG->INT STRLEN->STRSIZE in thousands places? >> Why not just define zend_long and zend_ulong to be 64-bit on 64-bit >> platforms and use them instead of int, ulint, zend_int, zend_uint, long, >> ulong where it's necessary. >> >> Anatol, I understood your point about catching incompatibility code at >> compile-time, but I'm not sure if the new features cost such huge code base >> changes. > > To catch 64bit issues at compile is immensely valuable. We had so many > issues in the past, some of them leading to security issues. It is > also a one time job, with a little extra effort for two years. > Besides the portability improvements, code review and correctness is > one of the goal of this RFC. I can't remember to know any widely known > and sane project relying on on other type for buffer length. PHP does > not have to be different :) > >> 1) 64-bit integers on Windows (they are already 64-bit on other systems) > > Almost all :) > >> 2) 64-bit string length. I don't think many people are interested in that. >> Fortunately, the patch doesn't change the zval size, so it shouldn't make a >> lot of harm. However, usage of "zend_size_t" instead of "int" is a bit >> annoying. I would change it into the same "zend_long" or "zend_ulong". > > It is not about being interested but preventing many security issues > as well, by default. The large buffer availability is a good side > effect. The implementation correctness is also drastically improved in > this case.
After a short chat with Dmitry, it seems that the only open questions are: - renaming . pros . 100% compile time checks, unvaluable . clarity, no magic as we have now . code correctness, any 64bit port guides cover things the way we implemented it . cons . renaming will create extra work for the next 2 years . annoying (while I can live with it given the benefits in the long run) - SAPIs We have discussed many times the removal of the dead SAPIs. I think it is time to remove them and keep only those maintained and actually working (some sapis are not even possible to use anymore) I think we can clear these questions during the vote phases, as options. If anyone has an idea how to keep the advantage brought by the renaming (compile checks, clarity, code correctness) without actually doing the renaming in extensions, please fire it :) Cheers,
-- Pierre @pierrejoye | http://www.libgd.org

Lester Caine

12 years ago
Pierre Joye wrote:
> I think we can clear these questions during the vote phases, as options. > > If anyone has an idea how to keep the advantage brought by the > renaming (compile checks, clarity, code correctness) without actually > doing the renaming in extensions, please fire it:)
Is this still being put forward for a PHP5.x build? Surely this along with a tidy up on base modules that support it would be much better used as a base for a new PHP6 plan? Extensions that do not support it can also be left out and we have a clean demarcation. One thing I would ask is just what 32bit platforms will remain supported. While desktop machines are generally 64bit these days, tablet and mobile devices do still tend to be 32bit. The ITX based systems I'm working with are still 32bit and while they do tend to run as clients to bigger servers, some support their own PHP platform. I already have to support different integer sizes anyway and assuming PHP is 32bit simplifies things but I'm a little unsure how a change to direct support for 64bit might affect that split?
-- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

Dmitry Stogov

12 years ago
Of course, we are not going to drop 32-bit support :) This patch affects only PHP on 64-bit systems. Thanks. Dmitry. On Thu, Jan 23, 2014 at 2:56 PM, Lester Caine <lester@lsces.co.uk> wrote:

Derick Rethans

12 years ago
On Thu, 23 Jan 2014, Dmitry Stogov wrote:
> Of course, we are not going to drop 32-bit support :) > This patch affects only PHP on 64-bit systems.
Really? I thought it would also make PHP's int type 64-bit even on 32-bit platforms. cheers, Derick

Dmitry Stogov

12 years ago
Hi Derick, It would be a significant performance degradation :) Just imagine how to implement 64-bit multiplication and division on 32-bit CPU. Thanks. Dmitry. On Thu, Jan 23, 2014 at 4:17 PM, Derick Rethans <derick@php.net> wrote:

Lester Caine

12 years ago
Dmitry Stogov wrote:
> Hi Derick, > > It would be a significant performance degradation :) > Just imagine how to implement 64-bit multiplication and division on 32-bit > CPU.
My own query was probably more to do with maintaining 32bit results where there is interaction with 32bit only systems. But perhaps the question should rather be 'Just what will be returned as 64bit where it currently is maintained as 32bit?' I was under the impression I got 32bit integers even on 64bit installations and had to use another method to return the 64bit values core to Firebird's generator system.
> Thanks. Dmitry. > > > On Thu, Jan 23, 2014 at 4:17 PM, Derick Rethans <derick@php.net> wrote: > >> On Thu, 23 Jan 2014, Dmitry Stogov wrote: >> >>> Of course, we are not going to drop 32-bit support :) >>> This patch affects only PHP on 64-bit systems. >> >> Really? I thought it would also make PHP's int type 64-bit even on >> 32-bit platforms.
-- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

Andrew Faulds

12 years ago
On 23/01/14 13:12, Dmitry Stogov wrote:
> It would be a significant performance degradation :) > Just imagine how to implement 64-bit multiplication and division on 32-bit > CPU.
I doubt as much. Aren't the vast majority of servers 64-bit anyway? And those that aren't are probably just 32-bit-mode 64-bit CPU servers, which wouldn't be slow, right?
-- Andrea Faulds http://ajf.me/

Nikita Popov

12 years ago
On Thu, Jan 23, 2014 at 11:56 AM, Lester Caine <lester@lsces.co.uk> wrote:
> Pierre Joye wrote: > >> I think we can clear these questions during the vote phases, as options. >> >> If anyone has an idea how to keep the advantage brought by the >> renaming (compile checks, clarity, code correctness) without actually >> doing the renaming in extensions, please fire it:) >> > > Is this still being put forward for a PHP5.x build? Surely this along with > a tidy up on base modules that support it would be much better used as a > base for a new PHP6 plan? Extensions that do not support it can also be > left out and we have a clean demarcation. >
I agree with Lester (for once ^^). I think this change is too large for PHP 5.6. While the releaseprocess RFC does not guarantee compatibility of our internal APIs, it does say that source compatibility *should* be preserved for minor versions. Until now we stuck with that rule: Every minor release a few APIs are changed, but "few" usually means something like three or four, not five hundred. If this patch is included in PHP 5.6 I think it is very likely that many extensions will not be updated or take a long while in updating. It's one thing to add two or three new ifdefs to support a new PHP release and something entirely else to change virtually all types in your code and verify that it is still safe with the new sizes. I remember how lack of proper APC support was seen as a major problem with adoption of PHP 5.4 - now imagine how lack of support by nearly all extensions would look like... As such I think that PHP 6 is a better place for this patch. PHP 6 will presumably also clean up / change many other internal APIs, so we can have all the major internal (and external ^^) breakage in one release. At that point it might also make sense for many people to support separate versions of extensions for PHP 5 and 6 to avoid the ifdef-hell. Nikita

Anatoliy Belsky

12 years ago
Hi Nikita, On Thu, January 23, 2014 15:29, Nikita Popov wrote:
> On Thu, Jan 23, 2014 at 11:56 AM, Lester Caine <lester@lsces.co.uk> > wrote: > > >> Pierre Joye wrote: >> >> >>> I think we can clear these questions during the vote phases, as >>> options. >>> >>> If anyone has an idea how to keep the advantage brought by the >>> renaming (compile checks, clarity, code correctness) without actually >>> doing the renaming in extensions, please fire it:) >>> >> >> Is this still being put forward for a PHP5.x build? Surely this along >> with a tidy up on base modules that support it would be much better used >> as a base for a new PHP6 plan? Extensions that do not support it can >> also be left out and we have a clean demarcation. >> > > I agree with Lester (for once ^^). I think this change is too large for > PHP > 5.6. While the releaseprocess RFC does not guarantee compatibility of our > internal APIs, it does say that source compatibility *should* be preserved > for minor versions. Until now we stuck with that rule: Every minor > release a few APIs are changed, but "few" usually means something like > three or four, not five hundred.
But the "a few rule" is a subset of "src compatibility should be kept if possible, while breakages are allowed". That just means there was no such big patches yet, and the actual "rule" was never fully exploited.
> If this patch is included in PHP 5.6 I think it is very likely that many > extensions will not be updated or take a long while in updating. It's one > thing to add two or three new ifdefs to support a new PHP release and > something entirely else to change virtually all types in your code and > verify that it is still safe with the new sizes.
I think it depends on how active an extension is supported and developed. For instance on PECL are still some extensions not even been ported to PHP5. From the todays experience, a port to the current patch might take from 3 hours to 3 days, depending on complexity. Say 3 days vs 5 months until final, together with the porting docs and tools, most of the really active and being in demand extensions will be so far or users will tear the devs apart :)
> I remember how lack of proper APC support was seen as a major problem > with adoption of PHP 5.4 - now imagine how lack of support by nearly all > extensions would look like...
It would be interesting to have some data, how much 5.5 is used nowadays. But generally it's not correct comparing APC with all extensions, as APC is a very special case diving so deep into core. Porting of a normal extension wrapping or not a library is much easier.
> As such I think that PHP 6 is a better place for this patch. PHP 6 will > presumably also clean up / change many other internal APIs, so we can have > all the major internal (and external ^^) breakage in one release. At > that point it might also make sense for many people to support separate > versions of extensions for PHP 5 and 6 to avoid the ifdef-hell. >
What I'm really afraid of hearing "PHP6" is the assotiation with the golem of the previous flop :) There are still trails of PHP6 macros in the sources and extensions. Like "in PHP6 everything will be much better", but really and when? PHP6, when it comes, will face the same compatibility and stability issues which will need solutions. Or even more, if it'll have all possible breakages. On the other hand, instead of having just 2 years of merge headaches, it's just moved to some undefined point in time with undefined amount of merge headaches. And at the end, will the golem come to life? IMHO the best way to stabilize a software is by using it. No offense meant, but that's what I'm really afraid of. Regards Anatol

Hannes Magnusson

12 years ago
On Thu, Jan 23, 2014 at 9:52 AM, Anatol Belski <ab@php.net> wrote:
>> If this patch is included in PHP 5.6 I think it is very likely that many >> extensions will not be updated or take a long while in updating. It's one >> thing to add two or three new ifdefs to support a new PHP release and >> something entirely else to change virtually all types in your code and >> verify that it is still safe with the new sizes. > I think it depends on how active an extension is supported and developed. > For instance on PECL are still some extensions not even been ported to > PHP5. From the todays experience, a port to the current patch might take > from 3 hours to 3 days, depending on complexity. Say 3 days vs 5 months > until final, together with the porting docs and tools, most of the really > active and being in demand extensions will be so far or users will tear > the devs apart :) >
If zpp/error_docref/*printf compatibility is not kept it will take a long time to update pecl extensions, and risk epic failure in the future when continuing the work on the extensions zpp (COMPAT ? "zyx" : "xyz", &foo, &bar, &baz) will in 1year turn into zpp (COMPAT ? "zyx|xz" : "xyz|z", &foo, &bar, &baz, &meh) and work just fine.. until you hit the perfect storm Its easy to migrate php-src exts since they don't have to care about compatibility with multiple php versions, but needing to support 5.3, 5.4, 5.5, 5.6, 5.7, 6.0 in a pecl extension will be nightmare Also, keep in mind most developers priority is not to work on supporting unreleased versions of PHP, but to keep maintaining the ext and adding features - so even though the guesstimate 5months "until final" is true, the extensions won't be updated until 5months _after_ 5.6.0 is out. -Hannes

Pierre Joye

12 years ago
On Thu, Jan 23, 2014 at 7:38 PM, Hannes Magnusson <hannes.magnusson@gmail.com> wrote:
> On Thu, Jan 23, 2014 at 9:52 AM, Anatol Belski <ab@php.net> wrote: > >>> If this patch is included in PHP 5.6 I think it is very likely that many >>> extensions will not be updated or take a long while in updating. It's one >>> thing to add two or three new ifdefs to support a new PHP release and >>> something entirely else to change virtually all types in your code and >>> verify that it is still safe with the new sizes. >> I think it depends on how active an extension is supported and developed. >> For instance on PECL are still some extensions not even been ported to >> PHP5. From the todays experience, a port to the current patch might take >> from 3 hours to 3 days, depending on complexity. Say 3 days vs 5 months >> until final, together with the porting docs and tools, most of the really >> active and being in demand extensions will be so far or users will tear >> the devs apart :) >> > > > If zpp/error_docref/*printf compatibility is not kept it will take a > long time to update pecl extensions, and risk epic failure in the > future when continuing the work on the extensions > > zpp (COMPAT ? "zyx" : "xyz", &foo, &bar, &baz) will in 1year turn into > zpp (COMPAT ? "zyx|xz" : "xyz|z", &foo, &bar, &baz, &meh) and work > just fine.. until you hit the perfect storm > > > Its easy to migrate php-src exts since they don't have to care about > compatibility with multiple php versions, but needing to support 5.3, > 5.4, 5.5, 5.6, 5.7, 6.0 in a pecl extension will be nightmare > > Also, keep in mind most developers priority is not to work on > supporting unreleased versions of PHP, but to keep maintaining the ext > and adding features - so even though the guesstimate 5months "until > final" is true, the extensions won't be updated until 5months _after_ > 5.6.0 is out.
We have ported many extensions (all actively used and maintained) for 5.5.0 before it was released. Some bugs appeared when used with 5.5 and was discovered after final, but that's expected and nothing we can do about it but test, test and test :) That's why I do not really buy this argument to be used against this RFC for 5.6, as if not 5.6, it will be the same with 5.6 and even worst for the hypothetical 6.
-- Pierre @pierrejoye | http://www.libgd.org

Anatoliy Belsky

12 years ago
Hi Hannes, On Thu, January 23, 2014 19:38, Hannes Magnusson wrote:
> On Thu, Jan 23, 2014 at 9:52 AM, Anatol Belski <ab@php.net> wrote: > > >>> If this patch is included in PHP 5.6 I think it is very likely that >>> many extensions will not be updated or take a long while in updating. >>> It's one >>> thing to add two or three new ifdefs to support a new PHP release and >>> something entirely else to change virtually all types in your code >>> and verify that it is still safe with the new sizes. >> I think it depends on how active an extension is supported and >> developed. For instance on PECL are still some extensions not even been >> ported to PHP5. From the todays experience, a port to the current patch >> might take from 3 hours to 3 days, depending on complexity. Say 3 days >> vs 5 months until final, together with the porting docs and tools, most >> of the really active and being in demand extensions will be so far or >> users will tear the devs apart :) >> > > > If zpp/error_docref/*printf compatibility is not kept it will take a > long time to update pecl extensions, and risk epic failure in the future > when continuing the work on the extensions > > zpp (COMPAT ? "zyx" : "xyz", &foo, &bar, &baz) will in 1year turn into zpp > (COMPAT ? "zyx|xz" : "xyz|z", &foo, &bar, &baz, &meh) and work > just fine.. until you hit the perfect storm >
it's up to vote now, RFC updated (multiple votes in the same RFC). I've prepared a change to zpp for any case http://pastie.org/8662202 . The spprinf and derivatives - keeping %d working with real int and %ld with real long looks unavoidable, as those will need to be used at some places. Here the portable way would be still using ZEND_INT_FMT instead of %pd and co. But this is actually primitive, like in vim :%s,%ld," ZEND_INT_FMT ",g Maybe some place will need to be cleaned after that, still it will do the job.
> > Its easy to migrate php-src exts since they don't have to care about > compatibility with multiple php versions, but needing to support 5.3, 5.4, > 5.5, 5.6, 5.7, 6.0 in a pecl extension will be nightmare > > > Also, keep in mind most developers priority is not to work on > supporting unreleased versions of PHP, but to keep maintaining the ext and > adding features - so even though the guesstimate 5months "until final" is > true, the extensions won't be updated until 5months _after_ 5.6.0 is out. >
The compat issues between 5.4 and 5.5 are less than now. Even the amout of breakage between 5.3 and 5.5 is less than now. However I'd really suggest you to try the porting guide, if you couldn't get an average extension compile in 1 hour or even less - too bad. Clear, some tricky places may come after that, that's where one would need to spend more time than on the routine "replace" task. Of course not talking about the calibre of exts like ext\intl and similar, that's not an average case. From what I saw now, the routine replacement task to just get an extension compiling with older codebase might be 10-20% of the whole porting time. In some cases it'll be everything one needs. The routine is always annoying, that's true :) But it might be not the worst issue one need to fix, if ever. I believe to have appropriately reflected the current concerns with the voting options. So then the direction of the porting process can be affected by everyone. Regards Anatol

Christopher Jones

12 years ago
On 01/23/2014 09:52 AM, Anatol Belski wrote:
> Hi Nikita, > > On Thu, January 23, 2014 15:29, Nikita Popov wrote:
>> If this patch is included in PHP 5.6 I think it is very likely that many >> extensions will not be updated or take a long while in updating. It's one >> thing to add two or three new ifdefs to support a new PHP release and >> something entirely else to change virtually all types in your code and >> verify that it is still safe with the new sizes. > I think it depends on how active an extension is supported and developed. > For instance on PECL are still some extensions not even been ported to > PHP5. From the todays experience, a port to the current patch might take > from 3 hours to 3 days, depending on complexity. Say 3 days vs 5 months > until final, together with the porting docs and tools, most of the really > active and being in demand extensions will be so far or users will tear > the devs apart :)
It will take time for extensions to catch up, regardless of which release the 64 bit changes are merged. This will have a short or medium term negative effect on PHP but I believe the type cleanup is an overall step forward. I recall the 5.4 changes took some time to filter down to various extensions, and also took effort from people who were not the current maintainers. The 64 bit project should attempt to minimize the scope of code changes needed, and be released when suitable user and extension maintainer expectations have been set. For reference, here is Anatol's mapping that allows extensions using the new types etc to compile with older PHP releases: https://github.com/php/php-src/blob/str_size_and_int64/compat/compat.h I would have leaned towards not requiring macro name changes, but I wouldn't vote against the RFC because of them. The current extension migration process is: - change the code to the new types and macros - update ssprintf formats - update ZPP formats - review and update any signed<->unsigned comparisons - review and update library type casts, range checks etc To allow the extension to also build with older PHP versions then: - compat.h is required - some #ifdefs are needed, e.g. for ZPP. Chris
-- christopher.jones@oracle.com http://twitter.com/ghrd Free PHP & Oracle book: http://www.oracle.com/technetwork/topics/php/underground-php-oracle-manual-098250.html

Christopher Jones

12 years ago
On 01/23/2014 12:36 PM, Christopher Jones wrote:
> For reference, here is Anatol's mapping that allows extensions using > the new types etc to compile with older PHP releases: > https://github.com/php/php-src/blob/str_size_and_int64/compat/compat.h > I would have leaned towards not requiring macro name changes, but I > wouldn't vote against the RFC because of them.
Anatol: what about creating the inverse of the compat.h #defines? Old code can include those definitions and wouldn't need as many changes? Chris
-- christopher.jones@oracle.com http://twitter.com/ghrd Free PHP & Oracle book: http://www.oracle.com/technetwork/topics/php/underground-php-oracle-manual-098250.html

Anatoliy Belsky

12 years ago
Hi Dmitry, greateful thanks for taking a look at this patch. On Thu, January 23, 2014 09:42, Dmitry Stogov wrote:
> I completely agree with Nikita. > Why to rename LONG->INT STRLEN->STRSIZE in thousands places? > Why not just define zend_long and zend_ulong to be 64-bit on 64-bit > platforms and use them instead of int, ulint, zend_int, zend_uint, long, > ulong where it's necessary. > > Anatol, I understood your point about catching incompatibility code at > compile-time, but I'm not sure if the new features cost such huge code > base changes.
Firstly it's the historical reason as while porting it "had" to break so one could easy see the relevant places. After that - it's also a transformation in the mind, as that int placeholders do not depend on a fixed datatype anymore.
> 1) 64-bit integers on Windows (they are already 64-bit on other systems) > 2) 64-bit string length. I don't think many people are interested in that. > Fortunately, the patch doesn't change the zval size, so it shouldn't > make a lot of harm. However, usage of "zend_size_t" instead of "int" is a > bit annoying. I would change it into the same "zend_long" or "zend_ulong". >
The original patch was for size_t only. With only that it were Linux/Unix only improvement, as size_t is 64 bit on Windows so it'd have to stay int or become just unsigned. Omitting the size_t change and doing int64 were only improvement on Windows. Adding both is the three-way improvement - Linux and Windows with biggest possible strings, Windows with 64 bit integers. So in fact, doing only one of those wouldn't IMHO justify all the effort. Additional Windows improvement "for free" is the whole file API exhausting, so large file objects and offsets. That's true, the possibility to process gigabytes of data in memory will not be needed every day, however the presence of it is something else. Like, why should I be interested on something not available anyway? Keeping size_t separated semantically is good for several reasons. It's clean with the specification. Should it come to 64 bit integer on 32 bit platform, it's easier to continue (merging size_t and ulong would break this option). And, one day it can come to 128 bit integers (for what reasons ever). I know, it's science fiction now, but wasn't a RAM size of 1Gb so 10 years ago? You never know. So size_t separated from unsigned int is a good thing imho and keeps some interesting options open for the future. At the end line, the new vs. old names is the last thing I personally would ultimately hang on, given the essential modification is in place. However the code clearly expressing what happens is something I'd call more appropriate with such a big substantial change. Best regards Anatol

Dmitry Stogov

12 years ago
Hi Anatol, On Thu, Jan 23, 2014 at 3:18 PM, Anatol Belski <ab@php.net> wrote:
> Hi Dmitry, > > greateful thanks for taking a look at this patch. > > On Thu, January 23, 2014 09:42, Dmitry Stogov wrote: > > I completely agree with Nikita. > > Why to rename LONG->INT STRLEN->STRSIZE in thousands places? > > Why not just define zend_long and zend_ulong to be 64-bit on 64-bit > > platforms and use them instead of int, ulint, zend_int, zend_uint, long, > > ulong where it's necessary. > > > > Anatol, I understood your point about catching incompatibility code at > > compile-time, but I'm not sure if the new features cost such huge code > > base changes. > Firstly it's the historical reason as while porting it "had" to break so > one could easy see the relevant places. After that - it's also a > transformation in the mind, as that int placeholders do not depend on a > fixed datatype anymore. >
historical reason of the patch itself doesn't matter :) If there's no significant reason to introduce new names lets use old ones. Even with new names anyone is able to change macros but not types. It's better to use some smarter compile-time protection. I would try to play with "compile time assert" (in GCC it must be even possible to use __builtin_types_compatible_b()). For example: #define CONCAT_TOKENS(a, b) a ## b #define EXPAND_THEN_CONCAT(a, b) CONCAT_TOKENS(a, b) #define COMPILE_TIME_ASSERT(expr) do { \ enum { EXPAND_THEN_CONCAT(ASSERT_line_,__LINE__) = 1 / !!(expr) }; \ } while(0) int main() { int a; COMPILE_TIME_ASSERT(__builtin_types_compatible_p(typeof(a), long)); COMPILE_TIME_ASSERT(sizeof(a) == 8); return 0; }
> > > 1) 64-bit integers on Windows (they are already 64-bit on other systems) > > 2) 64-bit string length. I don't think many people are interested in > that. > > Fortunately, the patch doesn't change the zval size, so it shouldn't > > make a lot of harm. However, usage of "zend_size_t" instead of "int" is a > > bit annoying. I would change it into the same "zend_long" or > "zend_ulong". > > > The original patch was for size_t only. With only that it were Linux/Unix > only improvement, as size_t is 64 bit on Windows so it'd have to stay int > or become just unsigned. Omitting the size_t change and doing int64 were > only improvement on Windows. Adding both is the three-way improvement - > Linux and Windows with biggest possible strings, Windows with 64 bit > integers. So in fact, doing only one of those wouldn't IMHO justify all > the effort. >
After some thoughts I think that usage of "size_t" is a good thing for the future support of X32 ABI. X86: sizeof(int)=4 sizeof(long)=4 sizeof(sizet_t)=4 X86-64 (Linux): sizeof(int)=4 sizeof(long)=8 sizeof(sizet_t)=8 X86-64 (Win64): sizeof(int)=4 sizeof(long)=4 sizeof(sizet_t)=8 X32: sizeof(int)=4 sizeof(long)=8 sizeof(sizet_t)=4
> > Additional Windows improvement "for free" is the whole file API > exhausting, so large file objects and offsets. > > That's true, the possibility to process gigabytes of data in memory will > not be needed every day, however the presence of it is something else. > Like, why should I be interested on something not available anyway? > > Keeping size_t separated semantically is good for several reasons. It's > clean with the specification. Should it come to 64 bit integer on 32 bit > platform, it's easier to continue (merging size_t and ulong would break > this option). And, one day it can come to 128 bit integers (for what > reasons ever). I know, it's science fiction now, but wasn't a RAM size of > 1Gb so 10 years ago? You never know. So size_t separated from unsigned int > is a good thing imho and keeps some interesting options open for the > future. >
Agreed.
> At the end line, the new vs. old names is the last thing I personally > would ultimately hang on, given the essential modification is in place. > However the code clearly expressing what happens is something I'd call > more appropriate with such a big substantial change. >
It's questionable. I' mot sure what names are better but big changes are always annoying. May be it makes sense to extend votiing with options ("with renaming" and "without renaming") Thanks. Dmitry.

Anatoliy Belsky

12 years ago
Hi Dmitry, On Thu, January 23, 2014 13:37, Dmitry Stogov wrote:
> historical reason of the patch itself doesn't matter :) If there's no > significant reason to introduce new names lets use old ones. > > Even with new names anyone is able to change macros but not types. > It's better to use some smarter compile-time protection. > I would try to play with "compile time assert" (in GCC it must be even > possible to use __builtin_types_compatible_b()). For example: > > > #define CONCAT_TOKENS(a, b) a ## b > #define EXPAND_THEN_CONCAT(a, b) CONCAT_TOKENS(a, b) > #define COMPILE_TIME_ASSERT(expr) do { \ > enum { EXPAND_THEN_CONCAT(ASSERT_line_,__LINE__) = 1 / !!(expr) }; \ > } while(0) > > > int main() { > int a; > > COMPILE_TIME_ASSERT(__builtin_types_compatible_p(typeof(a), long)); > COMPILE_TIME_ASSERT(sizeof(a) == 8); > return 0; } >
That is what also Gopal pointed me to with this live example https://github.com/bagder/curl/blob/master/include/curl/typecheck-gcc.h There's a similar functionality with Visual C++ http://msdn.microsoft.com/en-us/library/dd537655.aspx but I'm not sure it's C++ only. Will have to try. Generally why I left the idea - as it were only clean for new implementation, with the old implementation (that compat.h stuff) one would need to use #undef which looks like a hack. Another thing is that iterating through the param spec and comparing it with the passed args on compile time were another tricky job. Now where you say it, I think that check is at least worth a try and even being in the new PHP only it would be a great thing. So putting on the TODO, as the most stuff is ported now.
> >> >>> 1) 64-bit integers on Windows (they are already 64-bit on other >>> systems) 2) 64-bit string length. I don't think many people are >>> interested in >> that. >>> Fortunately, the patch doesn't change the zval size, so it shouldn't >>> make a lot of harm. However, usage of "zend_size_t" instead of "int" >>> is a bit annoying. I would change it into the same "zend_long" or >> "zend_ulong". >> >>> >> The original patch was for size_t only. With only that it were >> Linux/Unix >> only improvement, as size_t is 64 bit on Windows so it'd have to stay >> int or become just unsigned. Omitting the size_t change and doing int64 >> were only improvement on Windows. Adding both is the three-way >> improvement - Linux and Windows with biggest possible strings, Windows >> with 64 bit integers. So in fact, doing only one of those wouldn't IMHO >> justify all the effort. >> > > After some thoughts I think that usage of "size_t" is a good thing for > the future support of X32 ABI. > > X86: sizeof(int)=4 sizeof(long)=4 sizeof(sizet_t)=4 > X86-64 (Linux): sizeof(int)=4 sizeof(long)=8 sizeof(sizet_t)=8 > X86-64 (Win64): sizeof(int)=4 sizeof(long)=4 sizeof(sizet_t)=8 > X32: sizeof(int)=4 sizeof(long)=8 sizeof(sizet_t)=4 >
Ok, there is even more platform mess out here :)
> >> >> Additional Windows improvement "for free" is the whole file API >> exhausting, so large file objects and offsets. >> >> That's true, the possibility to process gigabytes of data in memory >> will not be needed every day, however the presence of it is something >> else. Like, why should I be interested on something not available >> anyway? >> >> Keeping size_t separated semantically is good for several reasons. It's >> clean with the specification. Should it come to 64 bit integer on 32 >> bit platform, it's easier to continue (merging size_t and ulong would >> break this option). And, one day it can come to 128 bit integers (for >> what reasons ever). I know, it's science fiction now, but wasn't a RAM >> size of 1Gb so 10 years ago? You never know. So size_t separated from >> unsigned int is a good thing imho and keeps some interesting options >> open for the future. >> > > Agreed. > > > >> At the end line, the new vs. old names is the last thing I personally >> would ultimately hang on, given the essential modification is in place. >> However the code clearly expressing what happens is something I'd call >> more appropriate with such a big substantial change. >> > > It's questionable. I' mot sure what names are better but big changes are > always annoying. May be it makes sense to extend votiing with options > ("with renaming" and > "without renaming") >
Yeah, independent from the concrete names, it's about keeping balance between the real functionality and annoyance. I've split options in the RFC into 4 separate votes so far. All yes/no options about the patch itself, zpp, renamed macros and SAPIs. As all that zend_*_t and php_*_t types didn't exist before, would you like suggest other names? Please see the RFC, there are also type replacements for off_t and struct stat. Thanks Anatol

Pierre Joye

12 years ago
Hi, On Jan 10, 2014 3:58 PM, "Anatol Belski" <ab@php.net> wrote:
> > Hi, > > https://wiki.php.net/rfc/size_t_and_int64 > > The discussion time has come. The work on the feature branch continues. > The current patch is stable enough to be discussed. >
For the record, we actually like to target 5.6 for this change. There was a little confusion due to the 5.6 timetable having been slightly shifted. Also the various changes we introduce has been discussed intensively (on this list on irc) to minimize the amount of changes required to support 64bit integer. Cheers, Pierre

Anatoliy Belsky

12 years ago
On Sat, January 11, 2014 15:43, Pierre Joye wrote:
> Hi, > > > On Jan 10, 2014 3:58 PM, "Anatol Belski" <ab@php.net> wrote: > >> >> Hi, >> >> >> https://wiki.php.net/rfc/size_t_and_int64 >> >> >> The discussion time has come. The work on the feature branch continues. >> The current patch is stable enough to be discussed. >> >> > > For the record, we actually like to target 5.6 for this change. There was > a little confusion due to the 5.6 timetable having been slightly shifted. > > Also the various changes we introduce has been discussed intensively (on > this list on irc) to minimize the amount of changes required to support > 64bit integer. > >
I've updated the RFC accordingly, also the possible vote choices. Regards Anatol

Anatoliy Belsky

12 years ago
On Sat, January 11, 2014 17:05, Anatol Belski wrote:
>> For the record, we actually like to target 5.6 for this change. There >> was a little confusion due to the 5.6 timetable having been slightly >> shifted. >> >> Also the various changes we introduce has been discussed intensively >> (on >> this list on irc) to minimize the amount of changes required to support >> 64bit integer. >> >> >> > I've updated the RFC accordingly, also the possible vote choices. >
One note yet - as we target 5.6, not to conceal were the hard test process running in background. Both x64 and x86 builds are tested involving CLI, Apache and opcache. The test summary is available under http://131.107.220.66/PFTT-Results/STR_SIZE_AND_INT64/ Regards Anatol

Yasuo Ohgaki

12 years ago
Hi Anatol, On Fri, Jan 10, 2014 at 11:58 PM, Anatol Belski <ab@php.net> wrote:
> https://wiki.php.net/rfc/size_t_and_int64 > > The discussion time has come. The work on the feature branch continues. > The current patch is stable enough to be discussed.
I haven't dive into details of this improvement, so I'm assuming the improvement is only for 64bit platforms from the title. There are number of subsystems that supports 64bit int. e.g. file system, PostgreSQL large object, time, etc. It's nicer if 64bit int is supported on 32bit platforms. What's the reason not to introduce larger int for 32bit platforms? Too much performance impact? Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

Andrew Faulds

12 years ago
On 13/01/14 21:57, Yasuo Ohgaki wrote:
> What's the reason not to introduce larger int for 32bit platforms? > Too much performance impact?
I've wondered this myself. It makes me also wonder if it's the case using 64-bit on 32-bit platforms is a little too slow, couldn't you just transparently store 32-bit and then switch to 64-bit if necessary?
-- Andrea Faulds http://ajf.me/

Anatoliy Belsky

12 years ago
Hi Andrea, On Mon, 2014-01-13 at 22:03 +0000, Andrea Faulds wrote:
> > On 13/01/14 21:57, Yasuo Ohgaki wrote: > > What's the reason not to introduce larger int for 32bit platforms? > > Too much performance impact? > > I've wondered this myself. It makes me also wonder if it's the case > using 64-bit on 32-bit platforms is a little too slow, couldn't you just > transparently store 32-bit and then switch to 64-bit if necessary? >
thanks for the idea, but it might be much more complex as one thinks. If one has to switch between 32 vs 64 bit in zval, it screams for API breach. For example with functions accepting integers by reference. Anyway, just a speculation right now :) Regards Anatol

Anatoliy Belsky

12 years ago
Hi Yasuo, On Tue, 2014-01-14 at 06:57 +0900, Yasuo Ohgaki wrote:
> Hi Anatol, > > On Fri, Jan 10, 2014 at 11:58 PM, Anatol Belski <ab@php.net> wrote: > > > https://wiki.php.net/rfc/size_t_and_int64 > > > > The discussion time has come. The work on the feature branch continues. > > The current patch is stable enough to be discussed. > > > I haven't dive into details of this improvement, so > I'm assuming the improvement is only for 64bit platforms from the title. > > There are number of subsystems that supports 64bit int. e.g. file system, > PostgreSQL large object, time, etc. It's nicer if 64bit int is supported on > 32bit platforms. > > What's the reason not to introduce larger int for 32bit platforms? > Too much performance impact? >
thanks for the interest on this. Such an idea existed at the start among others, as well as introducing a new zval member for 'int64' besides 'long'. The reasons for not doing it (now), also from the todays point of view: - much higher patch complexity - with more complexity the risk of not finishing the patch grows - worse compatibility with libraries - worse compatibility with existing extensions - 64 bit processors are widely available - well, the slowliness you mention, whereby i've no benchmarks at hand I would describe the current patch already as a quantum jump for PHP, whereby small steps are much safer. Having placeholders instead of fixed types is a good base to do 64 bit int available on 32 bit platform. However I guess bringing it up will take not less amount of woman/hours, if not more. One can see the current state as an intermediate goal, where it leads is another question. What comes with the patch now: - 64 bit int support across windows/linux - 64 bit file operations across windows/linux - size_t support for string length, that means string length can be handled with userspace integers - open perspectives for things like you mention I've read your Postgres approach and that's doable even currently inside 32 bit, too. There is such functionality here and there, like ext/date. Regards Anatol

Yasuo Ohgaki

12 years ago
Hi Anatol, On Tue, Jan 14, 2014 at 8:05 AM, Anatol Belski <ab@php.net> wrote:
> thanks for the interest on this. Such an idea existed at the start among > others, as well as introducing a new zval member for 'int64' besides > 'long'. The reasons for not doing it (now), also from the todays point > of view: > > - much higher patch complexity > - with more complexity the risk of not finishing the patch grows > - worse compatibility with libraries > - worse compatibility with existing extensions > - 64 bit processors are widely available > - well, the slowliness you mention, whereby i've no benchmarks at hand >
Thank you for your explanations. These are understandable reasons. Let's finish current proposal, then we may try to extend int type. If I finished my todo list for PHP, I'll join your project! Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net