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

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. On Wed, May 14, 2014 at 8:44 AM, Pierre Joye <pierre.php@gmail.com> wrote: > hi Dmitry. > > On Wed, May 14, 2014 at 12:52 AM, Dmitry Stogov <dmitry@zend.com> wrote: > > 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/a941d8cf6c731640b51f > > > > Actually, 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/php > >> p 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 >