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
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_namesDiff of the branch with old names to master
http://belski.net/phpz/str_size_and_int64_old_names.diffAn 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_int64The same ext how it looks now (not using the new zpp placeholders)
https://github.com/weltling/phurpleA 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#L47The sample ext diff to the current mainstream base
https://github.com/weltling/str_size_and_int64_example/compare/str_size_and_int64_revertWere 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
Hi Andrey,
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_namesDiff of the branch with old names to master
http://belski.net/phpz/str_size_and_int64_old_names.diffAn 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_int64The same ext how it looks now (not using the new zpp placeholders)
https://github.com/weltling/phurpleA 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#L47The sample ext diff to the current mainstream base
https://github.com/weltling/str_size_and_int64_example/compare/str_size_
and_int64_revertWere 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.
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
Anatol,
Hi Andrey,
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_namesDiff of the branch with old names to master
http://belski.net/phpz/str_size_and_int64_old_names.diffAn 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_int64The same ext how it looks now (not using the new zpp placeholders)
https://github.com/weltling/phurpleA 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#L47The sample ext diff to the current mainstream base
https://github.com/weltling/str_size_and_int64_example/compare/str_size_
and_int64_revertWere 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.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
Andrey,
Anatol,
Hi Andrey,
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_name
sDiff of the branch with old names to master
http://belski.net/phpz/str_size_and_int64_old_names.diffAn 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_int64The same ext how it looks now (not using the new zpp placeholders)
https://github.com/weltling/phurpleA 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#L47The sample ext diff to the current mainstream base
https://github.com/weltling/str_size_and_int64_example/compare/str_s
ize_ and_int64_revertWere 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.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);
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
Anatol,
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.
I meant on 64 bit vanilla PHP vs the branch
Regards
Anatol
Andrey
Hi Andrey,
Anatol,
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.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