# 64 bit RFC, #2 and #3 yes - Thread: https://externals.io/message/71919 - Original archive: https://news-web.php.net/php.internals/71919 - Started by Anatol Belski on 2014-02-01T12:30:31+00:00 - Messages: 7 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #71919 — Anatol Belski — 2014-02-01T12:30:31+00:00 URL: https://externals.io/message/71919#71919 Hi, as the concerns on the BC breach by zpp and macros changes are huge, we've invented the below to make the essential change only visible. This branches have zpp and macros change reverted (like #2 and #3 had resulted yes), only the necessary 64 improvement is in place. PHP core, zpp and macros reverted to the state of mainstream https://github.com/weltling/php-src/tree/str_size_and_int64_old_names Diff of the branch with old names to master http://belski.net/phpz/str_size_and_int64_old_names.diff An extension ported and fully worky with 5.3/4/5 and str_size_and_int64 branch, diff https://github.com/weltling/phurple/compare/str_size_and_int64 The same ext how it looks now (not using the new zpp placeholders) https://github.com/weltling/phurple A sample extension generated with ext_skel from str_size_and_int64 branch with several usage examples, worky also wit h5.3/4/5 https://github.com/weltling/str_size_and_int64_example/blob/master/str_size_and_int64.c#L47 The sample ext diff to the current mainstream base https://github.com/weltling/str_size_and_int64_example/compare/str_size_and_int64_revert Were this an acceptable tradeoff for this RFC to make it, one could still decide it in favor of 5.6. The worries and wishes are not reasonless, which is clear. However I think to strike a compromise is important to keep balance. Regards Anatol --- ## #72197 — Andrey Hristov — 2014-02-04T13:19:11+00:00 URL: https://externals.io/message/71919#72197 · In reply to: #71919 > Hi, > > as the concerns on the BC breach by zpp and macros changes are huge, we've > invented the below to make the essential change only visible. > > This branches have zpp and macros change reverted (like #2 and #3 had > resulted yes), only the necessary 64 improvement is in place. > > PHP core, zpp and macros reverted to the state of mainstream > https://github.com/weltling/php-src/tree/str_size_and_int64_old_names > > Diff of the branch with old names to master > http://belski.net/phpz/str_size_and_int64_old_names.diff > > > An extension ported and fully worky with 5.3/4/5 and str_size_and_int64 > branch, diff > https://github.com/weltling/phurple/compare/str_size_and_int64 > > The same ext how it looks now (not using the new zpp placeholders) > https://github.com/weltling/phurple > > > A sample extension generated with ext_skel from str_size_and_int64 branch > with several usage examples, worky also wit h5.3/4/5 > https://github.com/weltling/str_size_and_int64_example/blob/master/str_size_and_int64.c#L47 > > The sample ext diff to the current mainstream base > https://github.com/weltling/str_size_and_int64_example/compare/str_size_and_int64_revert > > > Were this an acceptable tradeoff for this RFC to make it, one could still > decide it in favor of 5.6. The worries and wishes are not reasonless, > which is clear. However I think to strike a compromise is important to > keep balance. > > Regards > > Anatol > > > > I saw 2 problems with the patch, in mysqlnd. After 2 instances I stopped reading but a parameter of type uint is being declared as php_int_t, a flag variable. Andrey --- ## #72198 — Anatol Belski — 2014-02-04T14:39:47+00:00 URL: https://externals.io/message/71919#72198 · In reply to: #72197 Hi Andrey, > > > > […] > > I saw 2 problems with the patch, in mysqlnd. After 2 instances I stopped > reading but a parameter of type uint is being declared as php_int_t, a flag > variable. I appreciate you could take a look at this. Could you point to the exact place where you see an issue? With the replacement of uint there are multiple reasons (and we was talking about it while being on the discussion about the mysqli_poll), two main of those - php_size_t should be used for string lengths - even flags coming from userland may cause overflow in 64 bit mode So uint to php_uint_t might be because of that, depending on what you're talking about. Thanks Anatol --- ## #72202 — Andrey Hristov — 2014-02-04T15:53:53+00:00 URL: https://externals.io/message/71919#72202 · In reply to: #72198 Anatol, > Hi Andrey, > > > > […] > > I appreciate you could take a look at this. Could you point to the exact > place where you see an issue? With the replacement of uint there are > multiple reasons (and we was talking about it while being on the > discussion about the mysqli_poll), two main of those > > - php_size_t should be used for string lengths > - even flags coming from userland may cause overflow in 64 bit mode > > So uint to php_uint_t might be because of that, depending on what you're > talking about. > > Thanks > > Anatol > -typedef enum_func_status (*func_mysqlnd_conn_data__tx_commit_or_rollback)(MYSQLND_CONN_DATA * conn, const zend_bool commit, const unsigned int flags, const char * const name TSRMLS_DC); +typedef enum_func_status (*func_mysqlnd_conn_data__tx_commit_or_rollback)(MYSQLND_CONN_DATA * conn, const zend_bool commit, const php_int_t flags, const char * const name TSRMLS_DC); The API needs unsigned integer, not signed one. mysql, mysqli and pdo should take care when they get signed integers and pass them appropriately. Here is more: - unsigned int char_minlen; - unsigned int char_maxlen; + php_size_t char_minlen; + php_size_t char_maxlen; even unsigned int is too much for this data but still, going size_t just is completely unnecessary, as the value will be between 1 and 4. However, as you don't know the inner workings it is excuseable. Still my standpoint that we inflate memory usage where it is not necessary at all. - Check bug #52891 : Wrong data inserted with mysqli/mysqlnd when using bind_param, value > LONG_MAX + Check bug #52891 : Wrong data inserted with mysqli/mysqlnd when using bind_param, value > `PHP_INT_MAX` Here this is a line in a comment. The bug has specific title and it hasn't changed, however the constant mentioned is changed. A bit annoying. There might be more, I gave a glance look at it. I'm curious to see how much memory real scripts will use with the branch. Best, Andrey --- ## #72205 — Anatol Belski — 2014-02-04T16:58:18+00:00 URL: https://externals.io/message/71919#72205 · In reply to: #72202 Andrey, > Anatol, > > > > > > […] > -typedef enum_func_status > (*func_mysqlnd_conn_data__tx_commit_or_rollback)(MYSQLND_CONN_DATA * > conn, const zend_bool commit, const unsigned int flags, const char * const > name TSRMLS_DC); > > +typedef enum_func_status > (*func_mysqlnd_conn_data__tx_commit_or_rollback)(MYSQLND_CONN_DATA * > conn, const zend_bool commit, const php_int_t flags, const char * const > name TSRMLS_DC); My bad, this one has to be php_uint_t. > The API needs unsigned integer, not signed one. mysql, mysqli and pdo > should take care when they get signed integers and pass them > appropriately. > > Here is more: > > > - unsigned int char_minlen; > - unsigned int char_maxlen; > + php_size_t char_minlen; > + php_size_t char_maxlen; > > > even unsigned int is too much for this data but still, going size_t just > is completely unnecessary, as the value will be between 1 and 4. However, > as you don't know the inner workings it is excuseable. Still my standpoint > that we inflate memory usage where it is not necessary at all. > Homogenous 64 bit variables allow the compiler to construct a better binary. That regards especially variables for loops, pointers and string length comparsion. Avoiding conversions from 32 vs 64 bit enables exhaustion of the architecture. Also, in many cases, one gets int or zval from userspace, and that value (say number or string length) has to bubble through various internal functions which often use different datatypes for the same, so an overflow is possible. Thus, the security aspect. I'd be glad if you could accomplish support on this matter. > - Check bug #52891 : Wrong data inserted with mysqli/mysqlnd when > using bind_param, value > LONG_MAX + Check bug #52891 : Wrong data > inserted with mysqli/mysqlnd when using bind_param, value > `PHP_INT_MAX` > > Here this is a line in a comment. The bug has specific title and it > hasn't changed, however the constant mentioned is changed. A bit annoying. > We can fix it. > There might be more, I gave a glance look at it. I'm curious to see how > much memory real scripts will use with the branch. > In fact, while 64 bit programs take more memory that 32 bit ones, the exact advantage of the 64 bit systems is that the amount of that memory is bigger. So comparing a program on 32 bit system with 2G (without extensions) and the same program on 64 bit system with 8G memory - while the program size itself increases, the relative increase opposing to the system memory is smaller. Significant memory increase is only to expect when using huge amounts of data split into small pieces, say you have millions of string buffers of 3 byte length - in that case it'll need the same amount of pointers which are 4 vs 8 bytes big on 64 bit. Stack variables are of less importance for that. Regards Anatol --- ## #72301 — Andrey Hristov — 2014-02-05T22:12:24+00:00 URL: https://externals.io/message/71919#72301 · In reply to: #72205 Anatol, > > […] > In fact, while 64 bit programs take more memory that 32 bit ones, the > exact advantage of the 64 bit systems is that the amount of that memory is > bigger. So comparing a program on 32 bit system with 2G (without > extensions) and the same program on 64 bit system with 8G memory - while > the program size itself increases, the relative increase opposing to the > system memory is smaller. Significant memory increase is only to expect > when using huge amounts of data split into small pieces, say you have > millions of string buffers of 3 byte length - in that case it'll need the > same amount of pointers which are 4 vs 8 bytes big on 64 bit. Stack > variables are of less importance for that. I meant on 64 bit vanilla PHP vs the branch > Regards > > Anatol > Andrey --- ## #72307 — Anatol Belski — 2014-02-05T23:39:00+00:00 URL: https://externals.io/message/71919#72307 · In reply to: #72301 Hi Andrey, > Anatol, > > > > […] > > I meant on 64 bit vanilla PHP vs the branch I've just took two checkouts I've got at hand (a lot of configure options, same in each, debug builds), 5.5 and str_size_and_int64 on Linux and made a simple comparsion. Running php -r 'while(true){usleep(300);}' and summing private+shared on both. The memory increase is less than 1%. On windows the same is about 30% increase, but that's clear as there grows everything - zval, int and string size. In fact, there's nothing to compare with on Windows. Of course, it'll be better to see with real apps. Just got no such tests (and things like uprofiler or xdebug aren't ported yet). I mean, it might take more in some cases, clear, but not something completely excessive. As function arguments go on stack (usually) and are freed after the function call. Or even they might be stored in registers, that were the same. In the worst case, if compiler decides to put some of them into the data or bss section, it will affect the size of the image itself. That's where we probably will need to pay more attention for optimizations. In this context an interesting question were to compare the 5.x branches by how they increase in memory usage, to say that or other increase were acceptable. Andrey, please continue your review. If you think some place is converted inappropriately, so let's improve it if it. There will be anyway some time needed for the fine tuning. Thanks Anatol