Hi,
as announced previously, the vote starts on May 13th and ends on May 20th.
https://wiki.php.net/rfc/size_t_and_int64_next#vote
The RFC is considered approved with 50%+1 acceptance. Happy voting :)
Best regards
Anatol
Anatol,
We discussed your patch in private and I showed you the big penalty it
makes...
I really, don't see, what do you like to achieve initiating voting right
after that. :(
I've just take a quick look over your initial patch for phpng at
https://gist.github.com/weltling/a941d8cf6c731640b51f
Actually, I would support only one idea from your patch - make IS_LONG to
be 64-bit on _WIN64.
Your zend_size_t related changes, in my opinion, makes little sense and
actually makes more harm. I recompiled phpng with your patch on Linux
x86_64 and got the following numbers:
zend_string size increased from 24 to 32 bytes
HashTable size increased from 56 to 72 bytes
zend_op_array size increased from 248 to 264 bytes
zend_class_entry size increased from 512 to 568 bytes
size of each opcode sizeof(zend_op) from 48 to 56 bytes
Anyone may recompile phpng (with and without patch) and then get these
number running
$ gdb sapi/cli/php
p sizeof(zend_string)
...
Can you imagine memory consumption difference on a large application?
More memory usage => more CPU cache misses => worse performance.
and what are the advantages? strings and class names > 2GB?
For me it's too big payment for useless feature.
-1
Thanks. Dmitry.
On Wed, May 14, 2014 at 12:51 AM, Anatol Belski anatol.php@belski.netwrote:
Hi,
as announced previously, the vote starts on May 13th and ends on May 20th.
https://wiki.php.net/rfc/size_t_and_int64_next#vote
The RFC is considered approved with 50%+1 acceptance. Happy voting :)
Best regards
Anatol
hi Dmitry.
Anatol,
We discussed your patch in private and I showed you the big penalty it
makes...
We discussed how we could best cooperate to get phpng and this patch
together to ease everyone's work.
I really, don't see, what do you like to achieve initiating voting right
after that. :(
Moving forward as you rejected the whole idea for no good reason or
based on numbers that cannot be taken as valid as this stage.
I've just take a quick look over your initial patch for phpng at
https://gist.github.com/weltling/a941d8cf6c731640b51fActually, I would support only one idea from your patch - make IS_LONG to
be 64-bit on _WIN64.
Again. This patch is about clean, safe, standard implementation for
64bit plaforms and modern compilers, by far not only for windows
(which has one more gain with this patch, portability). It brings a
lot of guards, as well as safe and clean implementations. This is
something we should have done a long time ago. PHP is only of the only
OSS language I know still relying on old types and using integer for
buffer length. This is a bad practice and a well known troubles
source. To reduce it to only windows is hardly a good move.
Your zend_size_t related changes, in my opinion, makes little sense and
actually makes more harm. I recompiled phpng with your patch on Linux
x86_64 and got the following numbers:zend_string size increased from 24 to 32 bytes
HashTable size increased from 56 to 72 bytes
zend_op_array size increased from 248 to 264 bytes
zend_class_entry size increased from 512 to 568 bytes
size of each opcode sizeof(zend_op) from 48 to 56 bytes
These numbers cannot be taken as valid or seriously at this stage.
Restructuring these structs will certainly reduce the delta. I did not
have the time to analyze phpng possible other improvements but I will
do it soon, and will also check with other people from the compiler
team if we can improve it a bit more as well.
But rejecting a safe and clean 64bit support for a prototype, even
promising, is wrong, in so many ways. See below.
Anyone may recompile phpng (with and without patch) and then get these
number running
$ gdb sapi/cli/phpp sizeof(zend_string)
...Can you imagine memory consumption difference on a large application?
More memory usage => more CPU cache misses => worse performance.
I can imagine a lot of things but at this point phpng is a very good
prototype with promising results. There is a good base idea and a lot
of changes in many places improving performance. However it is very
far away from being ready, APIs are very inconsistent and painful to
use (mix of zend_string and char* usage, remaing or removal which may
not make sense or make the code way too complicated).
and what are the advantages? strings and class names > 2GB?
For me it's too big payment for useless feature.
I do not think it is that useful for both of us to redo the
discussions about this patch and its goal. It is a necessary step for
64bit support. It is also very hard to use a prototype, developed for
months privately and still being in very early pre-alpha/testing phase
as argument to reject these changes. However, as I told you, it would
be way better to do both together, for everyone. But you reject the
idea. We have to move forward to php-next and I do not think we can
afford to wait months until phpng is remotely usable or stable (at
least APIs wised).
In any case, if the 64bit patch is accepted we will support you and
other with phpng as we always do, even before its RFC or proposal,
this is what I call cooperation and teamwork.
Cheers.
Pierre
hi Dmitry.
Anatol,
We discussed your patch in private and I showed you the big penalty it
makes...We discussed how we could best cooperate to get phpng and this patch
together to ease everyone's work.
I really, don't see, what do you like to achieve initiating voting right
after that. :(Moving forward as you rejected the whole idea for no good reason or
based on numbers that cannot be taken as valid as this stage.Your zend_size_t related changes, in my opinion, makes little sense and
actually makes more harm. I recompiled phpng with your patch on Linux
x86_64 and got the following numbers:zend_string size increased from 24 to 32 bytes
HashTable size increased from 56 to 72 bytes
zend_op_array size increased from 248 to 264 bytes
zend_class_entry size increased from 512 to 568 bytes
size of each opcode sizeof(zend_op) from 48 to 56 bytesThese numbers cannot be taken as valid or seriously at this stage.
Restructuring these structs will certainly reduce the delta. I did not
have the time to analyze phpng possible other improvements but I will
do it soon, and will also check with other people from the compiler
team if we can improve it a bit more as well.
Sorry, what did I miss here? Why cannot the phpng numbers be taken as
"valid"? The very same issue also exists in our current implementation. In
phpng the relative hit is just larger, because the structures are more
optimized.
I think you shouldn't dismiss Dmitry's point just like that. Having support
for 64 bit integers on Windows and other LLP64 architectures - that's
great. Making string lengths unsigned - that's great as well. But
supporting strings larger than 4G or arrays with more than 4 billion
elements - that does not seem very useful and unlike the other two changes,
hurts memory usage. I wonder how many people would prefer having lower
memory usage over having the ability to create arrays with 4 billion
elements.
Independently of that: In a lot of the previous discussion people have
many, many, many times asked that this patch be implemented without all
those macros renames and zpp changes. I still have a hard time seeing the
benefit of doing that. The zpp changes also conflict with phpng, because S
has a different meaning (and imho for no good reason - it could just as
well stay at s).
Nikita
Sorry, what did I miss here? Why cannot the phpng numbers be taken as
"valid"? The very same issue also exists in our current implementation. In
phpng the relative hit is just larger, because the structures are more
optimized.I think you shouldn't dismiss Dmitry's point just like that. Having support
for 64 bit integers on Windows and other LLP64 architectures - that's
great. Making string lengths unsigned - that's great as well. But
supporting strings larger than 4G or arrays with more than 4 billion
elements - that does not seem very useful and unlike the other two changes,
hurts memory usage. I wonder how many people would prefer having lower
memory usage over having the ability to create arrays with 4 billion
elements.Independently of that: In a lot of the previous discussion people have
many, many, many times asked that this patch be implemented without all
those macros renames and zpp changes. I still have a hard time seeing the
benefit of doing that. The zpp changes also conflict with phpng, because S
has a different meaning (and imho for no good reason - it could just as
well stay at s).Nikita
I don't have a vote on the RFC but I still have to say +1 on this one.
It makes sense to me to fix stuff that people might need in the next
5-10 years, but beyond that ???
IMO, Nikita has summarized the sensible threshold well.
Regards Terry
Sorry, what did I miss here? Why cannot the phpng numbers be taken as
"valid"?
At this stage, this is a key part of this sentence.
The very same issue also exists in our current implementation. In
phpng the relative hit is just larger, because the structures are more
optimized.I think you shouldn't dismiss Dmitry's point just like that.
I do not and did not. I would love to see the same from the phpng
side, but it failed, for no valid reason.
Having support
for 64 bit integers on Windows and other LLP64 architectures - that's great.
Making string lengths unsigned - that's great as well. But supporting
strings larger than 4G or arrays with more than 4 billion elements
Very large string or arrays are only side effects, it does not change
the goals and benefits of these changes.
- that
does not seem very useful and unlike the other two changes, hurts memory
usage. I wonder how many people would prefer having lower memory usage over
having the ability to create arrays with 4 billion elements.
This is a biased argument, you know it, I know it. The key point is
not about the new maximum size of an array or string but the long due
clean and safe 64bit implementation, following well known good
practice (can be seen in almost all other OSS projects out there) and
standards.
Independently of that: In a lot of the previous discussion people have many,
many, many times asked that this patch be implemented without all those
macros renames and zpp changes. I still have a hard time seeing the benefit
of doing that. The zpp changes also conflict with phpng, because S has a
different meaning (and imho for no good reason - it could just as well stay
at s).
This can be adapted, this is a details. It is also why I have tried
to get phpng and this patch along together and get both teams work
together. Cooperation in this case will be benefit for php as a whole
as more optimization can be achieve while keeping the safe&clean
implementation.
As of now, phpng has been worked on for the last months, totally
privately. And even if it looks promising it is still not remotely
ready to be actually proposed. However it does not prevent you to use
it to stop other improvements, which have been worked on for months,
publically, with continuous tests, status updates, etc. I am not sure
what is happening here is good for PHP.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
Independently of that: In a lot of the previous discussion people have many,
many, many times asked that this patch be implemented without all those
macros renames and zpp changes. I still have a hard time seeing the benefit
of doing that. The zpp changes also conflict with phpng, because S has a
different meaning (and imho for no good reason - it could just as well stay
at s).
This can be adapted, this is a details. It is also why I have tried
to get phpng and this patch along together and get both teams work
together. Cooperation in this case will be benefit for php as a whole
as more optimization can be achieve while keeping the safe&clean
implementation.As of now, phpng has been worked on for the last months, totally
privately. And even if it looks promising it is still not remotely
ready to be actually proposed. However it does not prevent you to use
it to stop other improvements, which have been worked on for months,
publically, with continuous tests, status updates, etc. I am not sure
what is happening here is good for PHP.
My personal impression is that phpng is yet another independent port of
php just like HHVM and the like. These all target a particular area of
PHP use and may not be suitable for 'home users'. As an alternative base
for PHPNext it may have a better pedigree and to that end a decision
needs to be made for the path forward. What seems totally out of place
here is a vote on something which has no real target yet? Has phpng
already been accepted as PHPNext? That PHPNext will be 64bit is a given?
So what is the need for a vote on a 'detail that can be changed'? It's
the detail elements that need to be agreed on ... not the principle of
64bit!
Hopefully there is no plan to backport this to the PHP5 builds?
--
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
Independently of that: In a lot of the previous discussion people have
many,
many, many times asked that this patch be implemented without all those
macros renames and zpp changes. I still have a hard time seeing the
benefit
of doing that. The zpp changes also conflict with phpng, because S has a
different meaning (and imho for no good reason - it could just as well
stay
at s).This can be adapted, this is a details. It is also why I have tried
to get phpng and this patch along together and get both teams work
together. Cooperation in this case will be benefit for php as a whole
as more optimization can be achieve while keeping the safe&clean
implementation.As of now, phpng has been worked on for the last months, totally
privately. And even if it looks promising it is still not remotely
ready to be actually proposed. However it does not prevent you to use
it to stop other improvements, which have been worked on for months,
publically, with continuous tests, status updates, etc. I am not sure
what is happening here is good for PHP.My personal impression is that phpng is yet another independent port of
php just like HHVM and the like. These all target a particular area of PHP
use and may not be suitable for 'home users'. As an alternative base for
PHPNext it may have a better pedigree and to that end a decision needs to
be made for the path forward. What seems totally out of place here is a
vote on something which has no real target yet? Has phpng already been
accepted as PHPNext? That PHPNext will be 64bit is a given? So what is the
need for a vote on a 'detail that can be changed'? It's the detail elements
that need to be agreed on ... not the principle of 64bit!Hopefully there is no plan to backport this to the PHP5 builds?
both the phpng and the size_t rfcs are targetting the next major version,
none of them are accepted yet, both of those would/will be suitable for
'home users'.
buth these are all public information, stated in the RFCs and discussed on
internals@ which you seems to be subscribed on based on your replies to the
list, so I'm not sure where the confusion comes from.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Hopefully there is no plan to backport this to the PHP5 builds?both the phpng and the size_t rfcs are targetting the next major
version, none of them are accepted yet, both of those would/will be
suitable for 'home users'.
buth these are all public information, stated in the RFCs and discussed
on internals@ which you seems to be subscribed on based on your replies
to the list, so I'm not sure where the confusion comes from.
Since original discussions were being targeted against PHP5 picking out
where discussions have moved on is not easily obvious. I seem to recall
this was originally targeted against PHP5?
Have we reached a point where PHP5 has been 'frozen' and so PHPNext is
the more detailed target today?
--
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
Hopefully there is no plan to backport this to the PHP5 builds?both the phpng and the size_t rfcs are targetting the next major
version, none of them are accepted yet, both of those would/will be
suitable for 'home users'.
buth these are all public information, stated in the RFCs and discussed
on internals@ which you seems to be subscribed on based on your replies
to the list, so I'm not sure where the confusion comes from.Since original discussions were being targeted against PHP5 picking out
where discussions have moved on is not easily obvious. I seem to recall
this was originally targeted against PHP5?Have we reached a point where PHP5 has been 'frozen' and so PHPNext is the
more detailed target today?
I guess you are referring to the previous rfc of the size_t, which was
targetting 5.6 and failed?
https://wiki.php.net/rfc/size_t_and_int64
many of the no voters mentioned that they would support the rfc but not in
a minor version hence the authors accepted and now tried to get in into the
next major, and here we are atm.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Hopefully there is no plan to backport this to the PHP5 builds?both the phpng and the size_t rfcs are targetting the next major
version, none of them are accepted yet, both of those would/will be
suitable for 'home users'.
buth these are all public information, stated in the RFCs and discussed
on internals@ which you seems to be subscribed on based on your replies
to the list, so I'm not sure where the confusion comes from.Since original discussions were being targeted against PHP5 picking out
where discussions have moved on is not easily obvious. I seem to recall
this was originally targeted against PHP5?Have we reached a point where PHP5 has been 'frozen' and so PHPNext is the
more detailed target today?I guess you are referring to the previous rfc of the size_t, which was
targetting 5.6 and failed?
https://wiki.php.net/rfc/size_t_and_int64
many of the no voters mentioned that they would support the rfc but not in
a minor version hence the authors accepted and now tried to get in into the
next major, and here we are atm.
Excactly. And we would not have done it if we knew that Zend was
working on a total rewamp. We would have rather focused on getting
phpng done faster, from a usability, design point of view, while
adding the necessary steps for a safe, clean and actual 64bit support.
But we did not know, nobody knew it until last week. But now we are
asked to trash months of work, openness and tests. Sorry, no.
Pierre
@pierrejoye | http://www.libgd.org
phpng is based on the same sources and 99% compatible.
We are just changing the basement.
it must be the basement for PHPNext, but we didn't start any discussions
about that.
We actually have a lot of work to do and spend most the time doing our best.
We have no plans to backport it into PHP-5.6.
PHP supports 64bit for ages, and this proposal has nothing common with
64bit support in general.
It allows 2GB strings, but do you imagine a web application that need them?
However, each big PHP site will have to "pay" for it.
Thanks. Dmitry.
Independently of that: In a lot of the previous discussion people have
many,
many, many times asked that this patch be implemented without all those
macros renames and zpp changes. I still have a hard time seeing the
benefit
of doing that. The zpp changes also conflict with phpng, because S has a
different meaning (and imho for no good reason - it could just as well
stay
at s).This can be adapted, this is a details. It is also why I have tried
to get phpng and this patch along together and get both teams work
together. Cooperation in this case will be benefit for php as a whole
as more optimization can be achieve while keeping the safe&clean
implementation.As of now, phpng has been worked on for the last months, totally
privately. And even if it looks promising it is still not remotely
ready to be actually proposed. However it does not prevent you to use
it to stop other improvements, which have been worked on for months,
publically, with continuous tests, status updates, etc. I am not sure
what is happening here is good for PHP.My personal impression is that phpng is yet another independent port of
php just like HHVM and the like. These all target a particular area of PHP
use and may not be suitable for 'home users'. As an alternative base for
PHPNext it may have a better pedigree and to that end a decision needs to
be made for the path forward. What seems totally out of place here is a
vote on something which has no real target yet? Has phpng already been
accepted as PHPNext? That PHPNext will be 64bit is a given? So what is the
need for a vote on a 'detail that can be changed'? It's the detail elements
that need to be agreed on ... not the principle of 64bit!Hopefully there is no plan to backport this to the PHP5 builds?
--
Lester Caine - G8HFLContact - 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
PHP supports 64bit for ages, and this proposal has nothing common with
64bit support in general.
It allows 2GB strings, but do you imagine a web application that need them?
However, each big PHP site will have to "pay" for it.
FWIW, it does concern general 64-bit support. It makes PHP act consistently across all 64-bit platforms.
Also, switching to size_t for string length does not just mean ‘2GB strings’. It means using the right tool for the job. Even the C standard library uses size_t for string length!
--
Andrea Faulds
http://ajf.me/
phpng is based on the same sources and 99% compatible.
You surely meant 99% incompatible right?
It changes literally every single line of code related to hashtable,
zval or related areas. It adds dozen of possible issues not easily
catch-able because of types change for numerous widely used APIs,
besides being inconsistent (yet, that's on the todos).
We are just changing the basement.
it must be the basement for PHPNext, but we didn't start any discussions
about that.
No, it is one task for php-next, one upcoming RFC (well, more in a few
months than next week).
We actually have a lot of work to do and spend most the time doing our best.
We have no plans to backport it into PHP-5.6.
So we do, and we do it now for phpng as well. As coop is the only way to go.
PHP supports 64bit for ages, and this proposal has nothing common with
64bit support in general.
This statement is wrong, you know it. I may re post common good
practices and recommendations for actual 64bit support but that will
be doubled.
It allows 2GB strings, but do you imagine a web application that need them?
As I already said numerous times, 2GB+ is a side effect, not a goal.
However, each big PHP site will have to "pay" for it.
and how many times do we have to pay for Zend total lack of clue about
open processes and communications?
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
Hi!
and how many times do we have to pay for Zend total lack of clue about
open processes and communications?
Really, this just derails the discussion and does not help anything. Can
we please stay in the bounds of professional conduct here?
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
phpng is based on the same sources and 99% compatible.
You surely meant 99% incompatible right?
It changes literally every single line of code related to hashtable,
zval or related areas. It adds dozen of possible issues not easily
catch-able because of types change for numerous widely used APIs,
besides being inconsistent (yet, that's on the todos).We are just changing the basement.
it must be the basement for PHPNext, but we didn't start any discussions
about that.No, it is one task for php-next, one upcoming RFC (well, more in a few
months than next week).We actually have a lot of work to do and spend most the time doing our best.
We have no plans to backport it into PHP-5.6.So we do, and we do it now for phpng as well. As coop is the only way to go.
PHP supports 64bit for ages, and this proposal has nothing common with
64bit support in general.This statement is wrong, you know it. I may re post common good
practices and recommendations for actual 64bit support but that will
be doubled.It allows 2GB strings, but do you imagine a web application that need them?
As I already said numerous times, 2GB+ is a side effect, not a goal.
However, each big PHP site will have to "pay" for it.
and how many times do we have to pay for Zend total lack of clue about
open processes and communications?
Pierre, you are getting the things wrong. Changing the types is
straightforward thing to do so an open branch. What the Zend guys did
was try and see approach with their ideas. If the results were bad you
would have probably never heard of phpng. This kind of things sometimes
are not open from the very beginning to limit the noise.
I hope I'm wrong but you are probably hurt because the size_t patch did
not get accepted and somehow now phpng gets lot of attention.
Cheers,
Andrey
Hi!
This is a biased argument, you know it, I know it. The key point is
not about the new maximum size of an array or string but the long due
clean and safe 64bit implementation, following well known good
practice (can be seen in almost all other OSS projects out there) and
standards.
I personally am still confused about why clean and safe 64-bit
implementation requires 64-bit string lengths.
As of now, phpng has been worked on for the last months, totally
privately. And even if it looks promising it is still not remotely
ready to be actually proposed. However it does not prevent you to use
it to stop other improvements, which have been worked on for months,
publically, with continuous tests, status updates, etc. I am not sure
what is happening here is good for PHP.
Nobody is proposing to stop other improvements. What people (including
myself) are concerned about is that change to 64-bit strings, due to the
increase in memory usage, may negate the very perceivable benefits of
the phpng and impact performance, all while not being beneficial for 99%
of the users. This is something that can not be fixed by saying "phpng
is not production quality yet". It is not, but we have an issue here
that does not depend on quality of phpng and we need to find a
resolution for it. Maybe if we have types cleaned up and
compartmentalized, we could have it working with both options and then
make a decision based on later performance tests?
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Well put Nikita!
Guys - we're in a bit of a ridiculous situation where the key low-level
engine maintainers are saying this patch is unacceptable, yet it may pass
due to the low level of overall interest and the lack of special rules to
govern low-level changes like that (where with all due respect, I think the
main maintainers of the engine should get much stronger power).
The patch the way it is now should be discarded; All the macro changes, as
well as any data structure change except for IS_LONG should be removed.
Given that the current RFC doesn't thoroughly represent the performance hit
(in terms of memory footprint, as well as resulting performance hit -
especially when using phpng), I recommend the following:
- Add the relevant performance feedback from Dmitry to the RFC, as well as
his concerns as the chief performance guy php.net has - Provide an option for people to vote 'yes' for the IS_LONG size part only
If the authors of the RFC object, my request from everyone who has voting
rights here is to vote 'no' and we can create a separate RFC for the IS_LONG
part only. Forcing this change against the explicit concerns of Dmitry and
Nikita, who worked their rear ends off to squeeze every bit of performance
in phpng (along with Xinchen, and now others joining in) - is, well,
ridiculous IMHO.
Zeev
Sorry, what did I miss here? Why cannot the phpng numbers be taken as
"valid"?
The very same issue also exists in our current implementation. In phpng
the
relative hit is just larger, because the structures are more optimized.I think you shouldn't dismiss Dmitry's point just like that. Having
support for 64
bit integers on Windows and other LLP64 architectures - that's great.
Making
string lengths unsigned - that's great as well. But supporting strings
larger than
4G or arrays with more than 4 billion elements - that does not seem very
useful
and unlike the other two changes, hurts memory usage. I wonder how many
people would prefer having lower memory usage over having the ability to
create arrays with 4 billion elements.Independently of that: In a lot of the previous discussion people have
many,
many, many times asked that this patch be implemented without all those
macros renames and zpp changes. I still have a hard time seeing the
benefit of
doing that. The zpp changes also conflict with phpng, because S has a
different
meaning (and imho for no good reason - it could just as well stay at s).Nikita
Well put Nikita!
Guys - we're in a bit of a ridiculous situation where the key low-level
engine maintainers are saying this patch is unacceptable, yet it may pass
due to the low level of overall interest and the lack of special rules to
govern low-level changes like that (where with all due respect, I think the
main maintainers of the engine should get much stronger power).The patch the way it is now should be discarded; All the macro changes, as
well as any data structure change except for IS_LONG should be removed.Given that the current RFC doesn't thoroughly represent the performance hit
(in terms of memory footprint, as well as resulting performance hit -
especially when using phpng), I recommend the following:
- Add the relevant performance feedback from Dmitry to the RFC, as well as
his concerns as the chief performance guy php.net has- Provide an option for people to vote 'yes' for the IS_LONG size part only
If the authors of the RFC object, my request from everyone who has voting
rights here is to vote 'no' and we can create a separate RFC for the IS_LONG
part only. Forcing this change against the explicit concerns of Dmitry and
Nikita, who worked their rear ends off to squeeze every bit of performance
in phpng (along with Xinchen, and now others joining in) - is, well,
ridiculous IMHO.Zeev
Sorry, what did I miss here? Why cannot the phpng numbers be taken as
"valid"?
The very same issue also exists in our current implementation. In phpng
the
relative hit is just larger, because the structures are more optimized.I think you shouldn't dismiss Dmitry's point just like that. Having
support for 64
bit integers on Windows and other LLP64 architectures - that's great.
Making
string lengths unsigned - that's great as well. But supporting strings
larger than
4G or arrays with more than 4 billion elements - that does not seem very
useful
and unlike the other two changes, hurts memory usage. I wonder how many
people would prefer having lower memory usage over having the ability to
create arrays with 4 billion elements.Independently of that: In a lot of the previous discussion people have
many,
many, many times asked that this patch be implemented without all those
macros renames and zpp changes. I still have a hard time seeing the
benefit of
doing that. The zpp changes also conflict with phpng, because S has a
different
meaning (and imho for no good reason - it could just as well stay at s).Nikita
I am not a developer, but I think the approach of the phpng developers is not fair. The 64 bit topic and its RFC has been worked on and discussed for weeks or months and now theirs suddenly phpng and all the former work should be thrown away?
I have not followed the whole discussion about 64 bit/size_t, etc, so I just want to ask if you, Nikita, Dmitry or Zeev have mentioned your view and the issues before?
Christian
On Wed, May 14, 2014 at 10:08 AM, Christian Stoller stoller@leonex.dewrote:
Well put Nikita!
Guys - we're in a bit of a ridiculous situation where the key low-level
engine maintainers are saying this patch is unacceptable, yet it may pass
due to the low level of overall interest and the lack of special rules to
govern low-level changes like that (where with all due respect, I think
the
main maintainers of the engine should get much stronger power).The patch the way it is now should be discarded; All the macro changes,
as
well as any data structure change except for IS_LONG should be removed.Given that the current RFC doesn't thoroughly represent the performance
hit
(in terms of memory footprint, as well as resulting performance hit -
especially when using phpng), I recommend the following:
- Add the relevant performance feedback from Dmitry to the RFC, as well
as
his concerns as the chief performance guy php.net has- Provide an option for people to vote 'yes' for the IS_LONG size part
onlyIf the authors of the RFC object, my request from everyone who has voting
rights here is to vote 'no' and we can create a separate RFC for the
IS_LONG
part only. Forcing this change against the explicit concerns of Dmitry
and
Nikita, who worked their rear ends off to squeeze every bit of
performance
in phpng (along with Xinchen, and now others joining in) - is, well,
ridiculous IMHO.Zeev
Sorry, what did I miss here? Why cannot the phpng numbers be taken as
"valid"?
The very same issue also exists in our current implementation. In phpng
the
relative hit is just larger, because the structures are more optimized.I think you shouldn't dismiss Dmitry's point just like that. Having
support for 64
bit integers on Windows and other LLP64 architectures - that's great.
Making
string lengths unsigned - that's great as well. But supporting strings
larger than
4G or arrays with more than 4 billion elements - that does not seem very
useful
and unlike the other two changes, hurts memory usage. I wonder how many
people would prefer having lower memory usage over having the ability to
create arrays with 4 billion elements.Independently of that: In a lot of the previous discussion people have
many,
many, many times asked that this patch be implemented without all those
macros renames and zpp changes. I still have a hard time seeing the
benefit of
doing that. The zpp changes also conflict with phpng, because S has a
different
meaning (and imho for no good reason - it could just as well stay at s).Nikita
I am not a developer, but I think the approach of the phpng developers is
not fair. The 64 bit topic and its RFC has been worked on and discussed for
weeks or months and now theirs suddenly phpng and all the former work
should be thrown away?I have not followed the whole discussion about 64 bit/size_t, etc, so I
just want to ask if you, Nikita, Dmitry or Zeev have mentioned your view
and the issues before?Christian
yeah, I think it would have been better to announce/share the phpng work a
bit sooner, but I think there is more than that in the current arguments:
the two RFCs have a conflict of interest, the phpng is focused on the
performance improvements through smaller memory usage, while the size_t
patch increases that, and the changes in phpng made the impact bigger.
So if we include both of those in their current form, the best scenario
which we can end up is clearing up the type system and introducing phpng
without any visible performance impact.
Of course this is only my understanding of the situation.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
yes. We discussed that patch with Pierre for hours, and I always told that
I afraid about memory consumption overhead.
my tests showed it clearly
phpng was started as closed project, because we didn't know if we'll able
to succeed at all (it's not a first attempt) and we liked to move fast.
Once, we got useful results we opened it.
see https://wiki.php.net/phpng#performance_evaluation
Thanks. Dmitry.
On Wed, May 14, 2014 at 12:08 PM, Christian Stoller stoller@leonex.dewrote:
Well put Nikita!
Guys - we're in a bit of a ridiculous situation where the key low-level
engine maintainers are saying this patch is unacceptable, yet it may pass
due to the low level of overall interest and the lack of special rules to
govern low-level changes like that (where with all due respect, I think
the
main maintainers of the engine should get much stronger power).The patch the way it is now should be discarded; All the macro changes,
as
well as any data structure change except for IS_LONG should be removed.Given that the current RFC doesn't thoroughly represent the performance
hit
(in terms of memory footprint, as well as resulting performance hit -
especially when using phpng), I recommend the following:
- Add the relevant performance feedback from Dmitry to the RFC, as well
as
his concerns as the chief performance guy php.net has- Provide an option for people to vote 'yes' for the IS_LONG size part
onlyIf the authors of the RFC object, my request from everyone who has voting
rights here is to vote 'no' and we can create a separate RFC for the
IS_LONG
part only. Forcing this change against the explicit concerns of Dmitry
and
Nikita, who worked their rear ends off to squeeze every bit of
performance
in phpng (along with Xinchen, and now others joining in) - is, well,
ridiculous IMHO.Zeev
Sorry, what did I miss here? Why cannot the phpng numbers be taken as
"valid"?
The very same issue also exists in our current implementation. In phpng
the
relative hit is just larger, because the structures are more optimized.I think you shouldn't dismiss Dmitry's point just like that. Having
support for 64
bit integers on Windows and other LLP64 architectures - that's great.
Making
string lengths unsigned - that's great as well. But supporting strings
larger than
4G or arrays with more than 4 billion elements - that does not seem very
useful
and unlike the other two changes, hurts memory usage. I wonder how many
people would prefer having lower memory usage over having the ability to
create arrays with 4 billion elements.Independently of that: In a lot of the previous discussion people have
many,
many, many times asked that this patch be implemented without all those
macros renames and zpp changes. I still have a hard time seeing the
benefit of
doing that. The zpp changes also conflict with phpng, because S has a
different
meaning (and imho for no good reason - it could just as well stay at s).Nikita
I am not a developer, but I think the approach of the phpng developers is
not fair. The 64 bit topic and its RFC has been worked on and discussed for
weeks or months and now theirs suddenly phpng and all the former work
should be thrown away?I have not followed the whole discussion about 64 bit/size_t, etc, so I
just want to ask if you, Nikita, Dmitry or Zeev have mentioned your view
and the issues before?Christian
yes. We discussed that patch with Pierre for hours,
one hour at best, and memory consuption impact was minimal when we
bench it using 5.6 as base. It can still be minimal using phpng as we
did not yet did the necessary work there, obviously (remember? you
guys announced it last week).
and I always told that
I afraid about memory consumption overhead.
my tests showed it clearlyphpng was started as closed project, because we didn't know if we'll able
to succeed at all (it's not a first attempt) and we liked to move fast.
That's your mistake. We are an opensource projects and ideas must be
(I repeat, must be) discussed early instead of very late. It works for
every other healthy OSS projects and I totally fail to see why Zend
and related cannot do it in a community friendly way.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
yes. We discussed that patch with Pierre for hours, and I always told that
I afraid about memory consumption overhead.
my tests showed it clearlyphpng was started as closed project, because we didn't know if we'll able
to succeed at all (it's not a first attempt) and we liked to move fast.
Once, we got useful results we opened it.
seehttps://wiki.php.net/phpng#performance_evaluation
Dmitry
As all of my systems are based on Firebird, I can't currently test even
if I did have the time. One area that I have problems with is the fact
that 64 bit numbers are a key element of SEQUENCE/GENERATOR values which
PHP does not support well and which I'd like to see fixed, but I can
understand your concern over 64bit strings increasing memory
consumption. This is why I think that lumping several 64bit related
items together does not make sense? Looking at this from the hardware
side, some 64bit processes will be faster on a 64bit machine, but also
supporting 32bit builds simply adds to the overheads. I'd prefer to see
better coverage of benchmarking since I don't think limited testing
gives the whole picture? But I don't see 64/32 changes having a
substantial effect. Do we need 64bit long strings - not generally - but
the facility should be covered properly while still retaining 32bit
operations?
--
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
Ah, you are on windows and lack 64-bit IS_LONG.
This is the part of the patch that should be accepted.
I mentioned it on original email.
The "bad" thing that this patch did, it changed all C data structures to
use 64-bit string lengths and it means that each such data sructure would
take more memory. Even zend_op becames bigger and as it's used for VM
byte-code representation you may just multiply the difference to number of
opcodes in application (that might be millions).
Unfortunately, phpng don't support firebird yet and it's not in our
priority list.
Thanks. Dmitry.
yes. We discussed that patch with Pierre for hours, and I always told that
I afraid about memory consumption overhead.
my tests showed it clearlyphpng was started as closed project, because we didn't know if we'll able
to succeed at all (it's not a first attempt) and we liked to move fast.
Once, we got useful results we opened it.
seehttps://wiki.php.net/phpng#performance_evaluationDmitry
As all of my systems are based on Firebird, I can't currently test even if
I did have the time. One area that I have problems with is the fact that 64
bit numbers are a key element of SEQUENCE/GENERATOR values which PHP does
not support well and which I'd like to see fixed, but I can understand your
concern over 64bit strings increasing memory consumption. This is why I
think that lumping several 64bit related items together does not make
sense? Looking at this from the hardware side, some 64bit processes will be
faster on a 64bit machine, but also supporting 32bit builds simply adds to
the overheads. I'd prefer to see better coverage of benchmarking since I
don't think limited testing gives the whole picture? But I don't see 64/32
changes having a substantial effect. Do we need 64bit long strings - not
generally - but the facility should be covered properly while still
retaining 32bit operations?--
Lester Caine - G8HFLContact - 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
Ah, you are on windows and lack 64-bit IS_LONG.
This is the part of the patch that should be accepted.
I mentioned it on original email.
Not used windows for a number of years. 64 bit builds on Linux ...
The "bad" thing that this patch did, it changed all C data structures to
use 64-bit string lengths and it means that each such data sructure
would take more memory. Even zend_op becames bigger and as it's used for
VM byte-code representation you may just multiply the difference to
number of opcodes in application (that might be millions).
Actually I do agree that this may not be ideal ...
Unfortunately, phpng don't support firebird yet and it's not in our
priority list.
Then there is no way that I can get involved at the present time :)
--
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
Ah, you are on windows and lack 64-bit IS_LONG.
This is the part of the patch that should be accepted.
I mentioned it on original email.Not used windows for a number of years. 64 bit builds on Linux ...
then you already have 64-bit long, or do I miss something?
The "bad" thing that this patch did, it changed all C data structures to
use 64-bit string lengths and it means that each such data sructure
would take more memory. Even zend_op becames bigger and as it's used for
VM byte-code representation you may just multiply the difference to
number of opcodes in application (that might be millions).Actually I do agree that this may not be ideal ...
Unfortunately, phpng don't support firebird yet and it's not in our
priority list.
Then there is no way that I can get involved at the present time :)
may be help in porting ext/interbase and ext/pdo_firebird :)
I really don't know a lot about Firebird and afraid even proper
configuration might take us significant time.
Thanks. Dmitry.
--
Lester Caine - G8HFLContact - 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
Not used windows for a number of years. 64 bit builds on Linux ...then you already have 64-bit long, or do I miss something?
BIGINT is not handled as a simple integer? But I do need a consistent
result if users are on windows which is an area where inconsistency has
been causing problems in the past.
Unfortunately, phpng don't support firebird yet and it's not in our priority list. Then there is no way that I can get involved at the present time :)may be help in porting ext/interbase and ext/pdo_firebird :)
I really don't know a lot about Firebird and afraid even proper
configuration might take us significant time.
Looks like you don't support Postgres either yet?
MySQL is the worst possible choice for any database application, so many
of us will not be able to follow you until these are supported, and
consistent handling of long strings/blobs and large integers is part of
the problem here.
While I'm trying to keep in touch with these new developments, until
I've managed to get all of my client base off PHP5.2 and up to 5.4 I
don't have time to spend on extra work. :(
( God I HATE the navigation crap on the top of the manual pages - this
should be a PROPER hierarchy of the page you are looking at not random
stuff you visited )
--
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
Hi,
from the previous episodes
Dmitry
http://grokbase.com/p/php/php-internals/141qd2k0ct/php-dev-rfc-64-bit-platform-improvements-for-string-length-and-integer
"After some thoughts I think that usage of "size_t" is a good thing for
the future support of X32 ABI."
Stas
http://grokbase.com/p/php/php-internals/1421be3ywy/php-dev-vote-64-bit-platform-improvements-for-string-length-and-integer
" I can't speak for others but I can speak for myself in saying
that these concerns are not just kicking the can down the road in hope
that eventually this patch dies off. "
Nikita:
http://www.serverphorums.com/read.php?7,862033,862730#msg-862730
"I fully support merging this into master, as the first change for PHP
6."
That's pretty sad guys. Applause for playing me for a fool (well, probably
not only me). On the other hand, I'd be honored to be as stupid as that
old farts from ANSI who wrote the specs.
Why can't you just work together instead of looking for a hair in the soup?
Best regards
Anatol
On Wed, May 14, 2014 at 11:39 PM, Lester Caine lester@lsces.co.uk
wrote:Ah, you are on windows and lack 64-bit IS_LONG.
This is the part of the patch that should be accepted.
I mentioned it on original email.Not used windows for a number of years. 64 bit builds on Linux ...
then you already have 64-bit long, or do I miss something?
The "bad" thing that this patch did, it changed all C data structures
touse 64-bit string lengths and it means that each such data sructure
would take more memory. Even zend_op becames bigger and as it's used
for VM byte-code representation you may just multiply the difference
to number of opcodes in application (that might be millions).Actually I do agree that this may not be ideal ...
Unfortunately, phpng don't support firebird yet and it's not in our
priority list.
Then there is no way that I can get involved at the present time :)
may be help in porting ext/interbase and ext/pdo_firebird :)
I really don't know a lot about Firebird and afraid even proper
configuration might take us significant time.Thanks. Dmitry.
--
Lester Caine - G8HFLContact - 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
For the benefit of everyone, this is all from January. Dmitry's,
Stas's and Nikita's positions in the actual patch in question can be
found on this thread.
Sent from my mobile
Hi,
from the previous episodes
Dmitry
http://grokbase.com/p/php/php-internals/141qd2k0ct/php-dev-rfc-64-bit-platform-improvements-for-string-length-and-integer
"After some thoughts I think that usage of "size_t" is a good thing for
the future support of X32 ABI."Stas
http://grokbase.com/p/php/php-internals/1421be3ywy/php-dev-vote-64-bit-platform-improvements-for-string-length-and-integer
" I can't speak for others but I can speak for myself in saying
that these concerns are not just kicking the can down the road in hope
that eventually this patch dies off. "Nikita:
http://www.serverphorums.com/read.php?7,862033,862730#msg-862730
"I fully support merging this into master, as the first change for PHP
6."That's pretty sad guys. Applause for playing me for a fool (well, probably
not only me). On the other hand, I'd be honored to be as stupid as that
old farts from ANSI who wrote the specs.Why can't you just work together instead of looking for a hair in the soup?
Best regards
Anatol
On Wed, May 14, 2014 at 11:39 PM, Lester Caine lester@lsces.co.uk
wrote:Ah, you are on windows and lack 64-bit IS_LONG.
This is the part of the patch that should be accepted.
I mentioned it on original email.
Not used windows for a number of years. 64 bit builds on Linux ...then you already have 64-bit long, or do I miss something?
The "bad" thing that this patch did, it changed all C data structures
touse 64-bit string lengths and it means that each such data sructure
would take more memory. Even zend_op becames bigger and as it's used
for VM byte-code representation you may just multiply the difference
to number of opcodes in application (that might be millions).
Actually I do agree that this may not be ideal ...Unfortunately, phpng don't support firebird yet and it's not in our
priority list.
Then there is no way that I can get involved at the present time :)may be help in porting ext/interbase and ext/pdo_firebird :)
I really don't know a lot about Firebird and afraid even proper
configuration might take us significant time.Thanks. Dmitry.
--
Lester Caine - G8HFLContact - 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
For the benefit of everyone, this is all from January. Dmitry's,
Stas's and Nikita's positions in the actual patch in question can be
found on this thread.
Stas position is not settled.
And in the meantime, from somewhere after January until last week, you
guys worked on a patch privately and did not consider a single second
that it could be a good thing for PHP to inform other about your work,
in progress or not. The memory increase is minimal against 5.6 (almost
at the measure error margin) and we do not have any number against
phpng, and won't be for months, until the actual re-factoring and
improvements are done. So excuse me Zeev, but this is not my vision
for the PHP core, and will never be. We, even as a team working under
a company umbrella, always follow community rules and try to cooperate
as much as we can, even working on things which are by no mean a
priority for us. I would love to see and hear the same from you.
Now. I am not saying that the changes in the core types will not
affect their respective size, it will, obviously (maybe less than
expected if we take a closer look at them). However the work on phpng
is in early stage, conceptual ideas have been proven good but
implementation is not yet finished and we have rooms for other
improvements. That makes the final impact of the 64bit patch, from 5.x
to 6.x, much less important as what it looks now.
Cheers,
Pierre
@pierrejoye | http://www.libgd.or
For the benefit of everyone, this is all from January. Dmitry's,
Stas's and Nikita's positions in the actual patch in question can be
found on this thread.
It should be known that I usually won't support Pierre, but the way this
discussion goes is really bad. There was a long debated and quite openly
developed patch. When it was initially proposed form my perspective the
result was "this might be good, timing is bad, we can't put it in 5.6"
Then some who participated in that debate had an idea and implemented,
ignored the existence of the public known work while working on
something contrary and none found the time to (publicly) discuss this.
This is really bad.
On the other side the attempt to push the size_t/int64 thing now looks
like an attempt to push it through now as a reaction, more for
"political" than technical reasons. This is bad, too. (While I
understand how ridiculous it is to first argue "now it is too late for
5.6" and then "oh, now it is too early for PHP.Next" as I'm essentially
doing)
What we as a community (both (engine) maintainers as well as user
communities) have to decide is how much performance loss we are willing
to pay for "clean" and "quality" code and how to find the balance. Maybe
an approach might be not using a single zend_string type everywhere but
have different types lie zend_small_string for usage in opcode members
(class names, function names, .. would almost certainly fit into a
(unsigned) char and are for the most part engine internal so places for
extra range checks should be minimal.
So please, let's try to avoid confrontation and let's try to define our
common goals and then finding a compromise for the technical questions.
johannes
We had a long discussion with Pierre on IRC and probably come to some
agreement.
However, we always talk as a blind with a deaf, so I can't be completely
sure :)
We agreed to make 64-bit IS_LONG and 64-bit string length on all 64-bit
systems, but don't change other core data structures (as Nikita suggested).
The main idea of the changes on top of phpng may be seen at
https://gist.github.com/dstogov/07fcbb60b1b585bcd290
I checked the patch on 64-bit Linux. The memory consumption grows, but no
so dramatic. On wordpress home page without opcache the peak_memory_usage()
showed 11043920 bytes insead of 10959456, that mean ~0.8% that we probably
can afford. Note that the original patch on the same test showed ~8% memory
consumption increase on master.
To integrate the patch into phpng we have to re-check and re-change every
place where zend_long, zend_ulong and string length are involved.
Pierre, if you agree with this proposal just change RFC accordingly, and
most people won't object.
I'm not sure if we need zend_ulong -> zend_int_t, IS_LONG -> IS_INT, size_t
-> zend_size_t renaming in thousand places. In my opinion this work is
useless but I won't object against patch just because of names. I would
suggest just 3 option for voting - "no", "yes with old names", "yes with
new names".
Thanks. Dmitry.
We agreed to make 64-bit IS_LONG and 64-bit string length on all 64-bit
systems, but don't change other core data structures (as Nikita suggested).
Does this mean we’re not switching to unsigned ints for other lengths? 64-bit or no, we ought to do that.
--
Andrea Faulds
http://ajf.me/
size_t is unsigned.
what do you mean by "other lengths"? (in phpng most data structures use
zend_string instead of char*/length, so we need to change it in one place).
May be some other places need to be changed as well, but I don't know about
them. If you talk about stream related structures, it must not make any
problems, because in run-time we will have just few such structures.
Thanks. Dmitry.
We agreed to make 64-bit IS_LONG and 64-bit string length on all 64-bit
systems, but don't change other core data structures (as Nikita
suggested).Does this mean we’re not switching to unsigned ints for other lengths?
64-bit or no, we ought to do that.--
Andrea Faulds
http://ajf.me/
size_t is unsigned.
what do you mean by "other lengths"? (in phpng most data structures use zend_string instead of char*/length, so we need to change it in one place).
Nevermind, looks like I was mistaken and we’re already using unsigned lengths everywhere.
Andrea Faulds
http://ajf.me/
Dmitry Stogov dmitry@zend.com schrieb:
--bcaec548a66d12428804f9c0c7f7
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printableWe had a long discussion with Pierre on IRC and probably come to some
agreement.
However, we always talk as a blind with a deaf, so I can't be completely
sure :)We agreed to make 64-bit IS_LONG and 64-bit string length on all 64-bit
systems, but don't change other core data structures (as Nikita suggested).The main idea of the changes on top of phpng may be seen at
https://gist.github.com/dstogov/07fcbb60b1b585bcd290I checked the patch on 64-bit Linux. The memory consumption grows, but no
so dramatic. On wordpress home page without opcache the peak_memory_usage()
showed 11043920 bytes insead of 10959456, that mean ~0.8% that we probably
can afford. Note that the original patch on the same test showed ~8% memory
consumption increase on master.
Sounds very good and 0.8% overhead is fine. Can we work on getting this
integrated into a v2 of the RFC, continue hopefully constructive discussions for
a week or two and then vote again. We aren't in a rush at the moment as
php.next is far away.
Sounds very good and 0.8% overhead is fine. Can we work on getting this
integrated into a v2 of the RFC, continue hopefully constructive discussions for
a week or two and then vote again. We aren't in a rush at the moment as
php.next is far away.
That sounds like a good reason to restart, the ML issues are good
reasons to begin it again (sigh). However I am not even sure the
issues are fixed by now... No activity on systems lately.
--
Pierre
@pierrejoye | http://www.libgd.org
Sounds very good and 0.8% overhead is fine. Can we work on getting this
integrated into a v2 of the RFC, continue hopefully constructive
discussions for
a week or two and then vote again. We aren't in a rush at the moment as
php.next is far away.That sounds like a good reason to restart, the ML issues are good
reasons to begin it again (sigh). However I am not even sure the
issues are fixed by now... No activity on systems lately.
This RFC is still listed as “in voting” on the wiki. What is its current
state?
--
Pierre@pierrejoye | http://www.libgd.org
Sounds very good and 0.8% overhead is fine. Can we work on getting this
integrated into a v2 of the RFC, continue hopefully constructive
discussions for
a week or two and then vote again. We aren't in a rush at the moment as
php.next is far away.That sounds like a good reason to restart, the ML issues are good
reasons to begin it again (sigh). However I am not even sure the
issues are fixed by now... No activity on systems lately.This RFC is still listed as “in voting” on the wiki. What is its current
state?
Bump.
Still listed as being in voting phase, and votes are still open.
Dmitry
http://grokbase.com/p/php/php-internals/141qd2k0ct/php-dev-rfc-64-bit-platform-improvements-for-string-length-and-integer
"After some thoughts I think that usage of "size_t" is a good thing for
the future support of X32 ABI.”
X32 ABI is an interesting one. It removes all the disadvantages of this patch. :)
Andrea Faulds
http://ajf.me/
Hi!
That's pretty sad guys. Applause for playing me for a fool (well, probably
not only me). On the other hand, I'd be honored to be as stupid as that
old farts from ANSI who wrote the specs.
I'm not sure how anything I said or did constitutes "playing you for a
fool". I still think it is a good thing to do - except for the part that
I voiced my objections to (and others did as well), and if we resolve
this objection, I certainly not hope the patch dies off, quite the
contrary. In fact, my mail makes even more sense now that we know that
evolutionary branch I've talked about without knowing if it will happen
- is actually happening in phpng. I'd certainly be happy to have the
changes brought into that branch - but we need to solve that memory
issue. I hope we can do it and find common ground - i.e. what Nikita
just recently proposed looks like a good way to go.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
I am not a developer, but I think the approach of the phpng developers is
not
fair. The 64 bit topic and its RFC has been worked on and discussed for
weeks or
months and now theirs suddenly phpng and all the former work should be
thrown away?
Dmitry communicated at least some of his concerns about this patch long ago,
back in January. Of course that's back when phpng didn't yet exist, and the
impact wasn't as big as it is now. This patch can undo partts of the gains
Dmitry and team managed to painstakingly squeeze out of the engine's data
structures.
You self-admitted not being a developer (a C developer I guess) - but I'm
sure you can imagine how painful it is to squeeze even 1% of performance
gain out of an already extremely optimized piece of code - one that's been
optimized for more than a decade - let alone 10-30%. This performance
currency shouldn't be traded lightly. Performance isn't above all other
things, but it is above changes which are purely theoretical and have no
meaningful end user value.
Zeev
Well put Nikita!
Guys - we're in a bit of a ridiculous situation where the key low-level
engine maintainers are saying this patch is unacceptable, yet it may pass
due to the low level of overall interest and the lack of special rules to
govern low-level changes like that (where with all due respect, I think the
main maintainers of the engine should get much stronger power).The patch the way it is now should be discarded; All the macro changes, as
well as any data structure change except for IS_LONG should be removed.Given that the current RFC doesn't thoroughly represent the performance hit
(in terms of memory footprint, as well as resulting performance hit -
especially when using phpng), I recommend the following:
- Add the relevant performance feedback from Dmitry to the RFC, as well as
his concerns as the chief performance guy php.net has- Provide an option for people to vote 'yes' for the IS_LONG size part only
If the authors of the RFC object, my request from everyone who has voting
rights here is to vote 'no' and we can create a separate RFC for the
IS_LONG
part only. Forcing this change against the explicit concerns of Dmitry and
Nikita, who worked their rear ends off to squeeze every bit of performance
in phpng (along with Xinchen, and now others joining in) - is, well,
ridiculous IMHO.Zeev
Even though that I agree, that our current voting process is suboptimal,
but I would rather see it fixed instead of ignoring the rules when it is
inconvenient/plain wrong.
What I do think is wrong with the current rfc, is that it states that it
needs only 50%+1 of the votes for acceptance.
The patch is big, it will affect every php installation (eg. not just some
platforms), it changes the Zend Engine, and it requires some non-trivial
effort from the extensions to support it, so even though that the direct
impact on the userland isn't that big (code with long strings etc. which
would fatal will now starts working, big numbers will overflow differently,
etc.) I think it still mandates a 66%+1 vote.
Just my 2 cents.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
I agree, and I’m not suggesting we ignore it; I just pointed out it makes
no sense that such a drastic low-level change will be forced on those who
effectively maintain the low-level engine for all of us. I don’t have a
magical solution on how to solve it at this point.
I did suggest two solutions to the situations within the current voting
process:
-
Clearly expressing the concerns Dmitry has about this patch in the
Performance section, so that at least people know what’s on the table; And
add another option – the option – the one that has the true userland
implications (the parts related to IS_LONG), and mention clearly that this
one will come with no meaningful performance (memory/CPU) drawbacks.
-
If the authors of the RFC object (which is within their right, of
course) – appealing to voters to vote ‘no’ on this one, and create another
RFC for the IS_LONG parts.
Zeev
Even though that I agree, that our current voting process is suboptimal,
but I would rather see it fixed instead of ignoring the rules when it is
inconvenient/plain wrong.
What I do think is wrong with the current rfc, is that it states that it
needs only 50%+1 of the votes for acceptance.
The patch is big, it will affect every php installation (eg. not just some
platforms), it changes the Zend Engine, and it requires some non-trivial
effort from the extensions to support it, so even though that the direct
impact on the userland isn't that big (code with long strings etc. which
would fatal will now starts working, big numbers will overflow differently,
etc.) I think it still mandates a 66%+1 vote.
Just my 2 cents.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Even though that I agree, that our current voting process is suboptimal,
but I would rather see it fixed instead of ignoring the rules when it is
inconvenient/plain wrong.
What I do think is wrong with the current rfc, is that it states that it
needs only 50%+1 of the votes for acceptance.
The patch is big, it will affect every php installation (eg. not just some
platforms), it changes the Zend Engine, and it requires some non-trivial
effort from the extensions to support it, so even though that the direct
impact on the userland isn't that big (code with long strings etc. which
would fatal will now starts working, big numbers will overflow differently,
etc.) I think it still mandates a 66%+1 vote.
I agree with Ferenc as far as the vote percentage is concerned. Though I
support this RFC, it seems pretty obvious that this represents a major,
fundamental change to the codebase, sufficient to warrant the 66%+1
requirement outlined in the RFC process. I hope it passes, but the RFC
should be updated to reflect the supermajority vote requirement, in my
opinion.
I'll admit I haven't been following this very closely, but it sounds to me
like the problem we're running into now was a lack of communication between
the Zend/main engine folks and Pierre's team. Aside from figuring out
where the blame for this lies (I'll leave that for others to argue), it
might not be a bad idea to come up with a way to ensure this sort of
disconnect never happens again.
I believe the RFC should be implemented in spite of these new concerns
because, either way, these changes are a step in the right direction and
are, in my opinion, a long time coming. Therefore, if that creates some
sort of conflict with recent work being done on the Zend side of things, I
think it would make the most sense for that to be where accommodations are
made rather than wasting all the work Pierre's group put into this project,
particularly since these objections, at least as I'm gathering from the
thread here, were never raised until fairly recently, after most of the
work had already been completed.
--Kris
I agree with Ferenc as far as the vote percentage is concerned. Though I
support this RFC, it seems pretty obvious that this represents a major,
fundamental change to the codebase, sufficient to warrant the 66%+1
requirement outlined in the RFC process. I hope it passes, but the RFC
should be updated to reflect the supermajority vote requirement, in my
opinion.
Why 66%+1, by the way? I can understand 50%+1, as that ensures a majority, but for 66% you already have a majority, so the +1 seems unreasonable.
--
Andrea Faulds
http://ajf.me/
Why 66%+1, by the way? I can understand 50%+1, as that ensures a majority,
but for 66% you already have a majority, so the +1 seems unreasonable.
That was my bad. It's not 66%+1. It's just 2/3.
--Kris
Why 66%+1, by the way? I can understand 50%+1, as that ensures a
majority, but for 66% you already have a majority, so the +1 seems
unreasonable.That was my bad. It's not 66%+1. It's just 2/3.
--Kris
Just a quick reminder, here's the document outlining the RFC voting process:
https://wiki.php.net/rfc/voting
--Kris
Just a quick reminder, here's the document outlining the RFC voting process:
While I don’t really disagree with 2/3 here, going by that RFC, isn’t this a 50%+1 non-language change?
--
Andrea Faulds
http://ajf.me/
Just a quick reminder, here's the document outlining the RFC voting process:
While I don’t really disagree with 2/3 here, going by that RFC, isn’t this a 50%+1 non-language change?
There is no language change.
--
Pierre
@pierrejoye | http://www.libgd.org
Just a quick reminder, here's the document outlining the RFC voting
process:While I don’t really disagree with 2/3 here, going by that RFC, isn’t
this a 50%+1 non-language change?There is no language change.
as I mentioned there are some direct effect on userland (different wrapping
of big numbers for example) which are ofc. intentional changes, but you
yourself also argued that changes which doesn't have direct effect on
userland should require 66% when it changes how the underlying engine
behaves:
http://grokbase.com/p/php/php-internals/13384w7z5c/php-dev-vote-integrating-zend-optimizer-into-the-php-distribution
http://grokbase.com/p/php/php-internals/1337w5353t/php-dev-vote-integrating-zend-optimizer-into-the-php-distribution
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Just a quick reminder, here's the document outlining the RFC voting process:
While I don’t really disagree with 2/3 here, going by that RFC, isn’t this a 50%+1 non-language change?
There is no language change.
if it's not language change than maybe leave the vote to extension
maintainers to vote whether it is suitable or not?
Best,
Andrey
Just a quick reminder, here's the document outlining the RFC voting
process:https://wiki.php.net/rfc/voting
While I don’t really disagree with 2/3 here, going by that RFC, isn’t this
a 50%+1 non-language change?
we tend to interpret "For these reasons, a feature affecting the language
itself (new syntax for example) will be considered as 'accepted' if it wins
a 2/3 of the votes. Other RFCs require 50% + 1 votes to get 'accepted'." in
a sense that anything which changes the php language (not just syntax, but
that is one example) to require a 2/3 majority.
if you interested in the pro/con arguments, you should check out the
opcache/Zend optimizer rfc/voting threads where we went through this the
last time(Opcache doesn't change the syntax, but changes the execution
paths which can affect potentially every php install).
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Just a quick reminder, here's the document outlining the RFC voting
process:https://wiki.php.net/rfc/voting
While I don’t really disagree with 2/3 here, going by that RFC, isn’t this
a 50%+1 non-language change?
Technically, I think it could probably be interpreted either way, depending
on how broadly you define "language change". Though the underlying syntax
of PHP isn't being altered, there are several macro renamings and numerous
other across-the-board changes. I think a reasonable argument could be
made that anything with this large of a scope could be defined as a
fundamental language change. I can't help but note that the voting RFC
mentions, "new syntax for example", the latter "for example" part seemingly
an acknowledgement that a language change is not limited solely to
syntactic alterations to the underlying grammar.
But again, I think a reasonable person could interpret the rule's language
either way. I would recommend that we update that RFC in the near future
to provide more specificity to account for otherwise subjective situations
like this. In the meantime, I suppose the decision is left to the RFC's
author, Anatol. If nothing else, I think it would be wise from a political
standpoint to err on the side of the higher requirement. Hopefully this
will get enough votes so the requirement will end-up being a moot point,
anyway.
--Kris
Folks:
I think it still mandates a 66%+1 vote.
Agreed. This is a major change.
Sincerely,
--Dan
--
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/
On Sat, May 17, 2014 at 6:30 AM, Daniel Convissor <
danielc@analysisandsolutions.com> wrote:
Folks:
I think it still mandates a 66%+1 vote.
Agreed. This is a major change.
Sincerely,
--Dan
--
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/--
I agree that it should have been, but under the current language of the
voting RFC, it can also be reasonably interpreted to call for a simple
majority. The RFC author chose to go with simple majority and left it
there. Trying to change the requirement mid-vote would be far more
troubling, in my view. Instead, we should discuss clarifying that language
in the voting RFC so that the interpretation is not so subjective in the
future.
--Kris
Hi,
Let's reiterate this over... 66%+1 of voting members and not 66%+1 of core
members. Is that right?
I really want this to be fixed, because Annotations for 66%+1 of voting
members, but not of core member and it got rejected.
Just to make things sure... I don't wanna hear about meritocracy again.
Cheers,
On Sat, May 17, 2014 at 6:30 AM, Daniel Convissor <
danielc@analysisandsolutions.com> wrote:Folks:
I think it still mandates a 66%+1 vote.
Agreed. This is a major change.
Sincerely,
--Dan
--
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/--
I agree that it should have been, but under the current language of the
voting RFC, it can also be reasonably interpreted to call for a simple
majority. The RFC author chose to go with simple majority and left it
there. Trying to change the requirement mid-vote would be far more
troubling, in my view. Instead, we should discuss clarifying that language
in the voting RFC so that the interpretation is not so subjective in the
future.--Kris
--
Guilherme Blanco
MSN: guilhermeblanco@hotmail.com
GTalk: guilhermeblanco
Toronto - ON/Canada
A few things:
-
We’re talking about 66%, not %66+1 (i.e., 20 against 10 means a
Yes, 19 against 10 means a no).
-
Nobody is arguing that language changes like annotations will be
decided by ‘Core’. It’s open to everyone who has a right to vote according
to the RFC process. Note that presently, people who are allowed to vote
technically don’t actually have a right to vote based on the Voting RFC,
which requires not only an SVN account, but also actual code contributions
to the PHP project; This was a known issue when we rolled out the voting
mechanism, but this limitation of the voting mechanism doesn’t change the
who’s eligible to vote.
-
I am arguing (as well as a few others) that implementation changes
– ones without a meaningful impact on the userbase at large, should be
decided by the respective developers of the code portion in question.
Annotations don’t fall under that category; Changing internal data
structures does, and so does changing a documentation platform or
implementation inside some PECL module. This isn’t simple to define but
I’m going to try and draft something up, probably based on the the Karma
assignment. Again, this will not impact people’s right to vote on the
vast majority of RFCs out there, which are almost always about features and
functions, and rarely about implementation. I still argue that the RFC
process was never meant to be about implementation, it was so outside the
scope of the RFC process that I didn’t even think about this possibility
when I helped drafted it.
Zeev
From: guilhermeblanco@gmail.com [mailto:guilhermeblanco@gmail.com]
Sent: Sunday, May 18, 2014 7:58 AM
To: Kris Craig
Cc: Daniel Convissor; Ferenc Kovacs; Zeev Suraski; Nikita Popov; PHP
Internals
Subject: Re: [PHP-DEV] [VOTE] [RFC] 2/3 vote needed (was: 64 bit platform
improvements...)
Hi,
Let's reiterate this over... 66%+1 of voting members and not 66%+1 of core
members. Is that right?
I really want this to be fixed, because Annotations for 66%+1 of voting
members, but not of core member and it got rejected.
Just to make things sure... I don't wanna hear about meritocracy again.
Cheers,
On Sat, May 17, 2014 at 6:30 AM, Daniel Convissor <
danielc@analysisandsolutions.com> wrote:
Folks:
I think it still mandates a 66%+1 vote.
Agreed. This is a major change.
Sincerely,
--Dan
--
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/--
I agree that it should have been, but under the current language of the
voting RFC, it can also be reasonably interpreted to call for a simple
majority. The RFC author chose to go with simple majority and left it
there. Trying to change the requirement mid-vote would be far more
troubling, in my view. Instead, we should discuss clarifying that language
in the voting RFC so that the interpretation is not so subjective in the
future.
--Kris
--
Guilherme Blanco
MSN: guilhermeblanco@hotmail.com
GTalk: guilhermeblanco
Toronto - ON/Canada
Am 18.05.2014 um 08:13 schrieb Zeev Suraski zeev@zend.com:
A few things:
We’re talking about 66%, not %66+1 (i.e., 20 against 10 means aYes, 19 against 10 means a no).
Just to be mathematically clear and to avoid confusion:
We are talking about a 2/3 majority meaning 66.66% not 66%. That means 66 against 34 means no even though it's 66%. Perhaps that's where the 66%+1 comes from.
Can't we simply talk of 2/3 to avoid any confusion?
Cheers
Andreas
Nobody is arguing that language changes like annotations will bedecided by ‘Core’. It’s open to everyone who has a right to vote according
to the RFC process. Note that presently, people who are allowed to vote
technically don’t actually have a right to vote based on the Voting RFC,
which requires not only an SVN account, but also actual code contributions
to the PHP project; This was a known issue when we rolled out the voting
mechanism, but this limitation of the voting mechanism doesn’t change the
who’s eligible to vote.
I am arguing (as well as a few others) that implementation changes– ones without a meaningful impact on the userbase at large, should be
decided by the respective developers of the code portion in question.
Annotations don’t fall under that category; Changing internal data
structures does, and so does changing a documentation platform or
implementation inside some PECL module. This isn’t simple to define but
I’m going to try and draft something up, probably based on the the Karma
assignment. Again, this will not impact people’s right to vote on the
vast majority of RFCs out there, which are almost always about features and
functions, and rarely about implementation. I still argue that the RFC
process was never meant to be about implementation, it was so outside the
scope of the RFC process that I didn’t even think about this possibility
when I helped drafted it.Zeev
From: guilhermeblanco@gmail.com [mailto:guilhermeblanco@gmail.com]
Sent: Sunday, May 18, 2014 7:58 AM
To: Kris Craig
Cc: Daniel Convissor; Ferenc Kovacs; Zeev Suraski; Nikita Popov; PHP
Internals
Subject: Re: [PHP-DEV] [VOTE] [RFC] 2/3 vote needed (was: 64 bit platform
improvements...)Hi,
Let's reiterate this over... 66%+1 of voting members and not 66%+1 of core
members. Is that right?I really want this to be fixed, because Annotations for 66%+1 of voting
members, but not of core member and it got rejected.Just to make things sure... I don't wanna hear about meritocracy again.
Cheers,
On Sat, May 17, 2014 at 6:30 AM, Daniel Convissor <
danielc@analysisandsolutions.com> wrote:Folks:
I think it still mandates a 66%+1 vote.
Agreed. This is a major change.
Sincerely,
--Dan
--
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/--
I agree that it should have been, but under the current language of the
voting RFC, it can also be reasonably interpreted to call for a simple
majority. The RFC author chose to go with simple majority and left it
there. Trying to change the requirement mid-vote would be far more
troubling, in my view. Instead, we should discuss clarifying that language
in the voting RFC so that the interpretation is not so subjective in the
future.--Kris
--
Guilherme Blanco
MSN: guilhermeblanco@hotmail.com
GTalk: guilhermeblanco
Toronto - ON/Canada
--
Andreas Heigl
Andreas@heigl.org
Can't we simply talk of 2/3 to avoid any confusion?
Agreed, especially considering that even 66.66% is technically inaccurate,
as is 66.66666666666%. It would be nice if the various character encodings
supported a repetend symbol somehow, but since they don't, 2/3 is really
the only sane way to express the requirement accurately.
--Kris
From: Kris Craig [mailto:kris.craig@gmail.com]
Sent: Sunday, May 18, 2014 10:27 AM
To: Andreas Heigl
Cc: Zeev Suraski; guilhermeblanco@gmail.com; Daniel Convissor; Ferenc
Kovacs; Nikita Popov; PHP Internals
Subject: Re: [PHP-DEV] [VOTE] [RFC] 2/3 vote needed (was: 64 bit platform
improvements...)
Can't we simply talk of 2/3 to avoid any confusion?
Agreed, especially considering that even 66.66% is technically inaccurate,
as is 66.66666666666%. It would be nice if the various character encodings
supported a repetend symbol somehow, but since they don't, 2/3 is really
the only sane way to express the requirement accurately.
Agreed as well. 2/3 is the language used by the current Voting RFC.
I still argue that the RFC
process was never meant to be about implementation, it was so outside the
scope of the RFC process that I didn’t even think about this possibility
when I helped drafted it.
There are three types of change to the code base that are of interest
here. I know that many consider the 'distributed' version as core. but
in reality many elements of that can be considered as extensions, and as
such people can still work with a much more compact distribution. So as
long as things like 'annotations' can be configured so that core code
does NOT require it to work, then discussions on that can be left to
those who have an interest. Until now unicode has been considered on
that basis and mbstring provides the 'namespace'. Moving multibyte
strings into mainstream code functionality has been happening but in a
piecemeal manor which like the 32/64bit discussion impacts a core set of
code, but few people actually understand the ramifications of 'little
changes' for which we rely on a core elite to advise where hidden mines
may be activated?
I am an electronics engineer by original training, and am still more at
home with low level coding on processors that one can actually
understand the operation of. The x86 family of processors moved outside
that frame a long time ago, and I would contest that few 'software
engineers' actually understand what they are programming on, so making
adjustments to how the core code interacts with the hardware running it
is something of a black art? It may have been relatively easy when there
was only AMD and Intel, but adding Via and other embedded x86
environments such as Atom brings a new set of mines. Throw into the mix
the vast range of similarly powered ARM based processors which do not
run x86 and we have a whole minefield?
Where do the changes being made in phpng fit into this tree? They are
attempting to reduce memory footprint so affect all the core API by
changing how structures are stored, and that may well be a good base to
build off, but on 64bit processors is it actually better to leave gaps
to correctly align 'words' that are 64bit based on these processors but
where packing two 32bit elements results in extra time to break these
apart? Certainly on the 64bit ARM processors correctly splitting
elements that are best processed in different registers can make a
difference. The main memory interface may be a bottleneck, but the
growing size of first line caches can offset that where large chunks of
code may be resident for a long period of time. That the compilers come
into this discussion is even more relevant since different compilers
will produce a different set of alignments? Handling a simple 10 byte
string could produce vastly different results depending on it's
alignment, and with 32 bit processors it was originally more efficient
to align to a 32bit boundary and pad to 12 bytes, so do these simple
rules map to the 64bit case? Reducing memory footprint may give gains in
one area, but at a cost in others? Alternatively would reducing the
large number of stings used for names of every type be more efficient
actually as 16bit string length? 10 years ago the answers were fairly
simple, but today does anybody actually know the answer?
Getting the core of a PHPNext right will be important and while I
understand why phpng has only recently been advertised given the amount
of work needed to get even it's limited application working, what is
needed is a much wider assessment that everybody can at least trial!
--
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
I agree that it should have been, but under the current language of the
voting RFC, it can also be reasonably interpreted to call for a simple
majority. The RFC author chose to go with simple majority and left it
there. Trying to change the requirement mid-vote would be far more
troubling, in my view. Instead, we should discuss clarifying that language
in the voting RFC so that the interpretation is not so subjective in the
future.
The reality is that under the current Voting RFC, it can be interpreted
either way. That said, the RFC authors are obviously biased by definition,
and there’s no reason to attribute more weight to their assessment of the
impact of the RFC than anybody else’s (not just in this case, but always).
That’s one of the issues with the current voting RFC.
Well put Nikita!
Guys - we're in a bit of a ridiculous situation
where the key low-level
engine maintainers are saying this patch is unacceptable,
What is amazingly disturbing, annoying and respectless is yet again
Zend, who are in my humble opinion not the only key developers since
very long, worked on a rewamp of the engine for months, without
sending a single notice to the list neither ask for feedback, support,
discussions about its design, etc.
yet it may pass
due to the low level of overall interest and the lack of special rules to
govern low-level changes like that (where with all due respect, I think the
main maintainers of the engine should get much stronger power).
Since when do you care about rules? Broke them for opcache last year
and it looks like it will happen again for this prototype.
The patch the way it is now should be discarded; All the macro changes, as
well as any data structure change except for IS_LONG should be removed.
So, if I understand correctly, you are asking to kill 80% of the
patch, reduce it to the only 64bit integer part. And what for? Because
you did not want to communicate earlier about your work on the engine?
Despite the numerous messages, discussions, status updates about many
tasks related to the next major version. Excuse me, but this is
actually not acceptable. Let alone the technical reasons why the 64bit
patch is a must have, since long.
Given that the current RFC doesn't thoroughly represent the performance hit
(in terms of memory footprint, as well as resulting performance hit -
especially when using phpng), I recommend the following:
Let me rephrase it without confusing other readers, hopefully.
phpng as it is now is a prototype. Very promising one but still a
prototype. It totally lacks any usable documentation to do tests, port
existing extensions or any real life tests, unlike the 64bit patch,
which provide everything and have been intensively tested since the
very first day, publicly, openly and every result has been discussed
here.
- Add the relevant performance feedback from Dmitry to the RFC, as well as
his concerns as the chief performance guy php.net has
Get phpng in a state where we can actually work with it, then we may
consider it. As you can see, we already provide support to bring it
there. But we are far away for being done, and I am only talking about
getting it up and running on common supported platforms. Design (APIs)
and other improvement still have to be done.
- Provide an option for people to vote 'yes' for the IS_LONG size part only
No. As it is not what we have to do. See the previous discussions and
my previous answers in this thread.
If the authors of the RFC object, my request from everyone who has voting
rights here is to vote 'no' and we can create a separate RFC for the IS_LONG
part only. Forcing this change against the explicit concerns of Dmitry and
Nikita, who worked their rear ends off to squeeze every bit of performance
in phpng (along with Xinchen, and now others joining in) - is, well,
ridiculous IMHO.
Learn to manage your team in a way that cooperation with php.net core
developers better. Performance is nice and shiny, but it should not be
done at the price of the overall healthiness of the PHP project. And
it becomes slowly a habit to push hard things for random reasons or
unstable/unfinished patches.
The proposal remains the same. I am more than convinced that we should
cooperate and get both in at the same time. There are always rooms for
improvements and we will provide support in many ways to achieve them,
as we always did. But asking to simply drop 80% of the patch for an
unknown and moving target is not acceptable.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
Well put Nikita!
Guys - we're in a bit of a ridiculous situation
where the key low-level
engine maintainers are saying this patch is unacceptable,What is amazingly disturbing, annoying and respectless is yet again
Zend, who are in my humble opinion not the only key developers since
very long, worked on a rewamp of the engine for months, without
sending a single notice to the list neither ask for feedback, support,
discussions about its design, etc.
today I spent few hours in discussions instead of planned work...
In case we started phpng as you suggest in the state when we didn't exactly
know what to do, we just didn't have it now.
And actually, not only Zend employees were involved in development.
Thanks. Dmitry.
yet it may pass
due to the low level of overall interest and the lack of special rules to
govern low-level changes like that (where with all due respect, I think
the
main maintainers of the engine should get much stronger power).Since when do you care about rules? Broke them for opcache last year
and it looks like it will happen again for this prototype.The patch the way it is now should be discarded; All the macro changes,
as
well as any data structure change except for IS_LONG should be removed.So, if I understand correctly, you are asking to kill 80% of the
patch, reduce it to the only 64bit integer part. And what for? Because
you did not want to communicate earlier about your work on the engine?
Despite the numerous messages, discussions, status updates about many
tasks related to the next major version. Excuse me, but this is
actually not acceptable. Let alone the technical reasons why the 64bit
patch is a must have, since long.Given that the current RFC doesn't thoroughly represent the performance
hit
(in terms of memory footprint, as well as resulting performance hit -
especially when using phpng), I recommend the following:Let me rephrase it without confusing other readers, hopefully.
phpng as it is now is a prototype. Very promising one but still a
prototype. It totally lacks any usable documentation to do tests, port
existing extensions or any real life tests, unlike the 64bit patch,
which provide everything and have been intensively tested since the
very first day, publicly, openly and every result has been discussed
here.
- Add the relevant performance feedback from Dmitry to the RFC, as well
as
his concerns as the chief performance guy php.net hasGet phpng in a state where we can actually work with it, then we may
consider it. As you can see, we already provide support to bring it
there. But we are far away for being done, and I am only talking about
getting it up and running on common supported platforms. Design (APIs)
and other improvement still have to be done.
- Provide an option for people to vote 'yes' for the IS_LONG size part
onlyNo. As it is not what we have to do. See the previous discussions and
my previous answers in this thread.If the authors of the RFC object, my request from everyone who has voting
rights here is to vote 'no' and we can create a separate RFC for the
IS_LONG
part only. Forcing this change against the explicit concerns of Dmitry
and
Nikita, who worked their rear ends off to squeeze every bit of
performance
in phpng (along with Xinchen, and now others joining in) - is, well,
ridiculous IMHO.Learn to manage your team in a way that cooperation with php.net core
developers better. Performance is nice and shiny, but it should not be
done at the price of the overall healthiness of the PHP project. And
it becomes slowly a habit to push hard things for random reasons or
unstable/unfinished patches.The proposal remains the same. I am more than convinced that we should
cooperate and get both in at the same time. There are always rooms for
improvements and we will provide support in many ways to achieve them,
as we always did. But asking to simply drop 80% of the patch for an
unknown and moving target is not acceptable.Cheers,
Pierre
@pierrejoye | http://www.libgd.org
I don't quite get this discussion in parts.
There are two conflicting proposals: one is growing data structures, one
is shrinking them. Obviously they clash.
There could be a third idea clashing with either one. It may lurk
somewhere on Github already but we just have not discovered it. The
third one may be that good, it may render all previous work null. Such
is life. I do know very well what it means to invest massive amounts of
time in things that never materialize. Extremly frustrating.
However, when judging the proposals, one may want to focus on technical
aspects.
Ulf
Hi!
- Add the relevant performance feedback from Dmitry to the RFC, as well as
his concerns as the chief performance guy php.net has- Provide an option for people to vote 'yes' for the IS_LONG size part only
I would gladly vote yes for types renaming too, even though zpp part is
a bit worrying, but we may end up rewriting zpp anyway soon (it's long
due) so it may not be that much of an issue. 64-bit lengths though are a
bit of an issue for me.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Pierre,
you developed a patch that will negatively affect every PHP application,
just to use "modern" types...
it's completely not necessary step for 64-bit support.
As I said, I can support only part of your patch related to IS_LONG size.
Thanks. Dmitry.
hi Dmitry.
Anatol,
We discussed your patch in private and I showed you the big penalty it
makes...We discussed how we could best cooperate to get phpng and this patch
together to ease everyone's work.I really, don't see, what do you like to achieve initiating voting right
after that. :(Moving forward as you rejected the whole idea for no good reason or
based on numbers that cannot be taken as valid as this stage.I've just take a quick look over your initial patch for phpng at
https://gist.github.com/weltling/a941d8cf6c731640b51fActually, I would support only one idea from your patch - make IS_LONG to
be 64-bit on _WIN64.Again. This patch is about clean, safe, standard implementation for
64bit plaforms and modern compilers, by far not only for windows
(which has one more gain with this patch, portability). It brings a
lot of guards, as well as safe and clean implementations. This is
something we should have done a long time ago. PHP is only of the only
OSS language I know still relying on old types and using integer for
buffer length. This is a bad practice and a well known troubles
source. To reduce it to only windows is hardly a good move.Your zend_size_t related changes, in my opinion, makes little sense and
actually makes more harm. I recompiled phpng with your patch on Linux
x86_64 and got the following numbers:zend_string size increased from 24 to 32 bytes
HashTable size increased from 56 to 72 bytes
zend_op_array size increased from 248 to 264 bytes
zend_class_entry size increased from 512 to 568 bytes
size of each opcode sizeof(zend_op) from 48 to 56 bytesThese numbers cannot be taken as valid or seriously at this stage.
Restructuring these structs will certainly reduce the delta. I did not
have the time to analyze phpng possible other improvements but I will
do it soon, and will also check with other people from the compiler
team if we can improve it a bit more as well.But rejecting a safe and clean 64bit support for a prototype, even
promising, is wrong, in so many ways. See below.Anyone may recompile phpng (with and without patch) and then get these
number running
$ gdb sapi/cli/phpp sizeof(zend_string)
...Can you imagine memory consumption difference on a large application?
More memory usage => more CPU cache misses => worse performance.I can imagine a lot of things but at this point phpng is a very good
prototype with promising results. There is a good base idea and a lot
of changes in many places improving performance. However it is very
far away from being ready, APIs are very inconsistent and painful to
use (mix of zend_string and char* usage, remaing or removal which may
not make sense or make the code way too complicated).and what are the advantages? strings and class names > 2GB?
For me it's too big payment for useless feature.I do not think it is that useful for both of us to redo the
discussions about this patch and its goal. It is a necessary step for
64bit support. It is also very hard to use a prototype, developed for
months privately and still being in very early pre-alpha/testing phase
as argument to reject these changes. However, as I told you, it would
be way better to do both together, for everyone. But you reject the
idea. We have to move forward to php-next and I do not think we can
afford to wait months until phpng is remotely usable or stable (at
least APIs wised).In any case, if the 64bit patch is accepted we will support you and
other with phpng as we always do, even before its RFC or proposal,
this is what I call cooperation and teamwork.Cheers.
Pierre
hi,
Pierre,
you developed a patch that will negatively affect every PHP application,
just to use "modern" types...
it's completely not necessary step for 64-bit support.
It is amazing that we have to argue about that in 2014.
Again, it is not only about about just using it but to finally have a
safe and clean 64bit implementation. Almost every other project did
that years ago, recommendation from various compilers developers,
security teams, etc. do recommend to implement 64bit support this way.
We had issues because of the lack of usage of this well known
standard. And now it is told that it is useless? I cannot agree with
this statement, sorry.
Now, about the numbers, which are based on a prototype, there are
rooms for more improvements, we both know that. As our numbers show,
based on 5.6 and 5.6+patch, the actual impact is much smaller. And
this is without digging deeper or without looking at what would be
possible when both phpng and this patch would be merged and improved
together. We are smart people and good developers, and we both have
very good contacts to compilers expert. We can reduce the impact even
more and improve the phpng optimization as well.
However, reducing the 64bit support to "only about allowing large
string", only to 64bit integer for long or "only about using modern
types" is wrong, and we again both know that.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
How can you reduce memory overhead if you start using 64-bit number for
class names and other entities?
If you have been developing this patch for months, why didn't you make it
already?
I'll tell you - because it's impossible.
Thanks. Dmitry.
hi,
Pierre,
you developed a patch that will negatively affect every PHP application,
just to use "modern" types...
it's completely not necessary step for 64-bit support.It is amazing that we have to argue about that in 2014.
Again, it is not only about about just using it but to finally have a
safe and clean 64bit implementation. Almost every other project did
that years ago, recommendation from various compilers developers,
security teams, etc. do recommend to implement 64bit support this way.
We had issues because of the lack of usage of this well known
standard. And now it is told that it is useless? I cannot agree with
this statement, sorry.Now, about the numbers, which are based on a prototype, there are
rooms for more improvements, we both know that. As our numbers show,
based on 5.6 and 5.6+patch, the actual impact is much smaller. And
this is without digging deeper or without looking at what would be
possible when both phpng and this patch would be merged and improved
together. We are smart people and good developers, and we both have
very good contacts to compilers expert. We can reduce the impact even
more and improve the phpng optimization as well.However, reducing the 64bit support to "only about allowing large
string", only to 64bit integer for long or "only about using modern
types" is wrong, and we again both know that.Cheers,
Pierre@pierrejoye | http://www.libgd.org
How can you reduce memory overhead if you start using 64-bit number for
class names and other entities?
If you have been developing this patch for months, why didn't you make it
already?
What I said is: rewamp the structs, which is something we did not want
to do before the work on the engine itself began, design, discussions,
etc.
We really did not expect to see you guys coming with this prototype
and pushing it hardly after having worked on it for months without
mentioning it or contacting us a single time. I blame us for being
cooperative and open. I will try to fix that, sorry.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
it is not only about about just using it but to finally have a
safe and clean 64bit implementation. Almost every other project did
that years ago, recommendation from various compilers developers,
security teams, etc. do recommend to implement 64bit support this way.
We had issues because of the lack of usage of this well known
standard.
Better security outweigh performance in general. Security and performance
is trade off relation almost always. Some structures/variables may use
32bit values for better performance when the gain outhweigh.
It seems we are better to reorganize RFC process. It would be better
agree on idea first, then details. Especially for large patches so that
people's
work would be wasted.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
as announced previously, the vote starts on May 13th and ends on May 20th.
https://wiki.php.net/rfc/size_t_and_int64_next#vote
The RFC is considered approved with 50%+1 acceptance. Happy voting :)
Actually, I have another question. Maybe I’m missing something, but anyway, what does the ‘phpng’ merge strategy on the vote mean?
Andrea Faulds
http://ajf.me/
as announced previously, the vote starts on May 13th and ends on May 20th.
https://wiki.php.net/rfc/size_t_and_int64_next#vote
The RFC is considered approved with 50%+1 acceptance. Happy voting :)
Actually, I have another question. Maybe I’m missing something, but anyway, what does the ‘phpng’ merge strategy on the vote mean?
Despite the blocking attempt happening now, we still think than
cooperation is better than trying to block other people work.
The idea is to merge the 64bit patch in phpng instead of master,
reducing the work for other developers while it will significantly
increase the amount of work for us.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
The idea is to merge the 64bit patch in phpng instead of master,
reducing the work for other developers while it will significantly
increase the amount of work for us.
Ah, I see. That’s what I wanted.
--
Andrea Faulds
http://ajf.me/
I've made some measurement in comparison with master branch.
The patch definitely increase memory consumption up to 8%.
It doesn't affect performance significantly (2-3% difference is a typical
measurement mistake)
for phpng I just may to compare the base structure sizes, because
str_size_and_int64 has to be adopted for phpng first.
Thanks. Dmitry.
base structure sizes [bytes]
phpng str_size_and_int64 diff
zval 16 16 0.00%
zend_string 24 32 33.33%
HashTable 56 72 28.57%
Bucket 32 32 0.00%
zend_op_array 248 264 6.45%
zend_class_entry 512 568 10.94%
zend_op 48 56 16.67%
base structure sizes [bytes]
master str_size_and_int64 diff
zval 24 24 0.00%
HashTable 72 80 11.11%
Bucket 72 72 0.00%
zend_op_array 248 272 9.68%
zend_class_entry 576 608 5.56%
zend_op 48 56 16.67%
peak memory usage of real life applications [bytes] (with opcache
disabled)
master str_size_and_int64 diff
Wordpress-3.6.0 11,899,256 12,875,168 8.20%
ZF1 29,287,976 31,131,864 6.30%
test performance [sec]
master str_size_and_int64 diff
bench.php 1.976 1.987 -0.56%
micro_bench.php 8.742 8.757 -0.17%
real-life performance [req/sec]
master str_size_and_int64 diff
blog 99.4 99.2 -0.20%
drupal 1581.6 1547.6 -2.15%
fw 202.2 197.9 -2.13%
hello 12247.2 12276.8 0.24%
qdig 488.9 490.2 0.27%
typo3 559.0 550.4 -1.54%
wordpress 177.9 178.3 0.22%
xoops 127.3 127.7 0.31%
scrum 173.3 173.9 0.35%
ZF1 Hello 1061.3 1060.4 -0.08%
ZF2 Test 173.0 167.3 -3.29%
wordpress-3.6 196.1 198.0 0.97%
On Wed, May 14, 2014 at 12:51 AM, Anatol Belski anatol.php@belski.netwrote:
Hi,
as announced previously, the vote starts on May 13th and ends on May 20th.
https://wiki.php.net/rfc/size_t_and_int64_next#vote
The RFC is considered approved with 50%+1 acceptance. Happy voting :)
Best regards
Anatol
I've made some measurement in comparison with master branch.
The patch definitely increase memory consumption up to 8%.
Up to means very little.
Average usage will help more. I will see if we have time to provide
updated numbers (did not change much since the last we provided).
Cheers,
Pierre
base structure sizes [bytes]
phpng str_size_and_int64 diff
zval 16 16 0.00%
zend_string 24 32 33.33%
HashTable 56 72 28.57%
Bucket 32 32 0.00%
zend_op_array 248 264 6.45%
zend_class_entry 512 568 10.94%
zend_op 48 56 16.67%base structure sizes [bytes]
master str_size_and_int64 diff
zval 24 24 0.00%
HashTable 72 80 11.11%
Bucket 72 72 0.00%
zend_op_array 248 272 9.68%
zend_class_entry 576 608 5.56%
zend_op 48 56 16.67%
Interesting, what’s going on here? It seems to have a worse effect on phpng HashTable than on master’s. Does the struct need re-aligning again, perhaps? Or did it just fit neatly into an existing blank space in master’s HashTable?
—
Andrea Faulds
http://ajf.me/
phpng optimizes core data structures and primitives especially to reduce
memory consumption and transfer.
The effect of the patch on phpng might be worse than on master, because it
does opposite things.
Thanks. Dmitry.
base structure sizes [bytes]
phpng str_size_and_int64 diff
zval 16 16 0.00%
zend_string 24 32 33.33%
HashTable 56 72 28.57%
Bucket 32 32 0.00%
zend_op_array 248 264 6.45%
zend_class_entry 512 568 10.94%
zend_op 48 56 16.67%base structure sizes [bytes]
master str_size_and_int64 diff
zval 24 24 0.00%
HashTable 72 80 11.11%
Bucket 72 72 0.00%
zend_op_array 248 272 9.68%
zend_class_entry 576 608 5.56%
zend_op 48 56 16.67%Interesting, what’s going on here? It seems to have a worse effect on
phpng HashTable than on master’s. Does the struct need re-aligning again,
perhaps? Or did it just fit neatly into an existing blank space in master’s
HashTable?
—
Andrea Faulds
http://ajf.me/
phpng optimizes core data structures and primitives especially to reduce
memory consumption and transfer.
The effect of the patch on phpng might be worse than on master, because it
does opposite things.
No, it does clean implementation and make it safe. Further improvements
with phpng can be done but it was obviously not possible before, because
phpng did not even exist from a php.net POV.
Thanks. Dmitry.
base structure sizes [bytes]
phpng str_size_and_int64 diff
zval 16 16 0.00%
zend_string 24 32 33.33%
HashTable 56 72 28.57%
Bucket 32 32 0.00%
zend_op_array 248 264 6.45%
zend_class_entry 512 568 10.94%
zend_op 48 56 16.67%base structure sizes [bytes]
master str_size_and_int64 diff
zval 24 24 0.00%
HashTable 72 80 11.11%
Bucket 72 72 0.00%
zend_op_array 248 272 9.68%
zend_class_entry 576 608 5.56%
zend_op 48 56 16.67%Interesting, what’s going on here? It seems to have a worse effect on
phpng HashTable than on master’s. Does the struct need re-aligning
again,
perhaps? Or did it just fit neatly into an existing blank space in
master’s
HashTable?
—
Andrea Faulds
http://ajf.me/
Hi!
phpng optimizes core data structures and primitives especially to reduce
memory consumption and transfer.
The effect of the patch on phpng might be worse than on master, because it
does opposite things.
Maybe we could optimize it by using different lengths for data and for
language-based strings? One can plausibly argue you can have 2G string
in zval, but having 2G class name is clearly out of the bounds of
reality. In fact, for class names even single byte would probably be
enough, but short should cover it with ample space. I understand that
may mean some complications in the code, so not sure if it's really a
good approach, just wanted to put it on the table to consider. If we
have places that need big strings and places that need short strings,
maybe we could have two types? Just an idea, not sure if it's not stupid :)
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi!
phpng optimizes core data structures and primitives especially to reduce
memory consumption and transfer.
The effect of the patch on phpng might be worse than on master, because it
does opposite things.Maybe we could optimize it by using different lengths for data and for
language-based strings? One can plausibly argue you can have 2G string
in zval, but having 2G class name is clearly out of the bounds of
reality. In fact, for class names even single byte would probably be
enough,
You have a point here, for class, variable and co names, everything is
in place already and does not need size_t. That's easy to change.
Everything else (zval and co) needs it tho.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org